-
Notifications
You must be signed in to change notification settings - Fork 152
Redirect legacy Dev Wiki /code.ext imports to .ext #16235
base: dev
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be worth to add a test for the logic:
- it entails adding both the file housing the code to be tested + the test file to https://github.com/Wikia/app/blob/dev/tests/karma/js-unit.conf.js
- existing tests like https://github.com/Wikia/app/blob/dev/extensions/wikia/GlobalShortcuts/scripts/spec/PageActions.spec.js can be an inspiration
- tests can be run by running
npm test
(after fetching the dependencies)
…lesheet URLs and respect wgScriptPath in their imports as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mszabo-wikia I added tests now (based on BannerNotifications.spec.js), does it look okay and should I cover more code in it?
@@ -138,20 +161,6 @@ window.appendCSS = function( text ) { | |||
return s; | |||
}; | |||
|
|||
// Special stylesheet links for Monobook only (see bug 14717) | |||
var skinpath = mw.config.get( 'stylepath' ) + '/' + mw.config.get( 'skin' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These aren't used anymore and don't seem like they will cause issues for custom scripts.
@@ -715,31 +724,32 @@ function getLabelFor (obj_id) { | |||
return false; | |||
} | |||
|
|||
if (skin != 'monaco' && skin != 'oasis') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This never executes anymore.
skins/common/wikibits.js
Outdated
// Check protocol-relative, HTTP and HTTPS versions of Dev Wiki links | ||
// This is done via wgWikiaBaseDomain so fandom.com migration does | ||
// not affect it and can't use wgWikiaBaseDomainRegex so it does | ||
// not potentially affect devbox URLs such as dev.wikia-dev.pl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any ideas for smarter implementation of this? Will using wgWikiaBaseDomain
still be an issue after the fandom.com migration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would consider using wgWikiaBaseDomainRegex
instead to cover both domains.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That will also cover URLs like dev.wikia-dev.pl
. Is that an issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, that's fine, makes it easier to have the same behaviour in our development environment too. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
describe('wikibits', function() { | ||
'use strict'; | ||
mw.config = new mw.Map(); | ||
var url1 = '/index.php?title=MediaWiki:UserTags/code.js&action=raw&ctype=text/javascript'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any smarter way of storing all these URLs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally I wouldn't store them using names url1, url2 etc..., those names lack context and they should be at least named according to what they store. Going further maybe a good idea would be to set a map like:
{
'UserTagsCodeJsRelative': {
'CodePartRemoved': '....',
'CodePartRemovedWithDomain':'....'
}
}
Using those keys should be easier to read and there would be no need to look for the values.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Couple months late, but public note of technical blockers:
|
var url1expect = '/index.php?title=MediaWiki:UserTags.js&action=raw&ctype=text/javascript'; | ||
var url1expect2 = '//dev.wikia.com/index.php?title=MediaWiki:UserTags.js&action=raw&ctype=text/javascript'; | ||
var url2 = '/index.php?title=User:TK-999/common.js&action=raw&ctype=text/javascript'; | ||
var url3 = 'http://dev.wikia.com/index.php?title=MediaWiki:UserTags/code.js&action=raw&ctype=text/javascript'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dev.wikia.com is moved to dev.fandom.com, also fandom.com works on https
var url3 = 'http://dev.wikia.com/index.php?title=MediaWiki:UserTags/code.js&action=raw&ctype=text/javascript'; | |
var url3 = 'https://dev.fandom.com/index.php?title=MediaWiki:UserTags/code.js&action=raw&ctype=text/javascript'; |
mw.config = new mw.Map(); | ||
var url1 = '/index.php?title=MediaWiki:UserTags/code.js&action=raw&ctype=text/javascript'; | ||
var url1expect = '/index.php?title=MediaWiki:UserTags.js&action=raw&ctype=text/javascript'; | ||
var url1expect2 = '//dev.wikia.com/index.php?title=MediaWiki:UserTags.js&action=raw&ctype=text/javascript'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var url1expect2 = '//dev.wikia.com/index.php?title=MediaWiki:UserTags.js&action=raw&ctype=text/javascript'; | |
var url1expect2 = '//dev.fandom.com/index.php?title=MediaWiki:UserTags.js&action=raw&ctype=text/javascript'; |
var url1expect2 = '//dev.wikia.com/index.php?title=MediaWiki:UserTags.js&action=raw&ctype=text/javascript'; | ||
var url2 = '/index.php?title=User:TK-999/common.js&action=raw&ctype=text/javascript'; | ||
var url3 = 'http://dev.wikia.com/index.php?title=MediaWiki:UserTags/code.js&action=raw&ctype=text/javascript'; | ||
var url3expect = '//dev.wikia.com/index.php?title=MediaWiki:UserTags/code.js&action=raw&ctype=text/javascript'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var url3expect = '//dev.wikia.com/index.php?title=MediaWiki:UserTags/code.js&action=raw&ctype=text/javascript'; | |
var url3expect = '//dev.fandom.com/index.php?title=MediaWiki:UserTags/code.js&action=raw&ctype=text/javascript'; |
var url2 = '/index.php?title=User:TK-999/common.js&action=raw&ctype=text/javascript'; | ||
var url3 = 'http://dev.wikia.com/index.php?title=MediaWiki:UserTags/code.js&action=raw&ctype=text/javascript'; | ||
var url3expect = '//dev.wikia.com/index.php?title=MediaWiki:UserTags/code.js&action=raw&ctype=text/javascript'; | ||
var url4 = 'https://dev.wikia.com/index.php?title=MediaWiki:UserTags/code.js&action=raw&ctype=text/javascript'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var url4 = 'https://dev.wikia.com/index.php?title=MediaWiki:UserTags/code.js&action=raw&ctype=text/javascript'; | |
var url4 = 'https://dev.fandom.com/index.php?title=MediaWiki:UserTags/code.js&action=raw&ctype=text/javascript'; |
var url5 = 'http://platform.twitter.com/widgets.js'; | ||
var url6 = '/index.php?title=MediaWiki:WikiaNavigationBarStyle/code.css&action=raw&ctype=text/css'; | ||
var url6expect = '/index.php?title=MediaWiki:WikiaNavigationBarStyle.css&action=raw&ctype=text/css'; | ||
var url7 = 'http://dev.wikia.com/index.php?title=MediaWiki:WikiaNavigationBarStyle/code.css&action=raw&ctype=text/css'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var url7 = 'http://dev.wikia.com/index.php?title=MediaWiki:WikiaNavigationBarStyle/code.css&action=raw&ctype=text/css'; | |
var url7 = 'https://dev.fandom.com/index.php?title=MediaWiki:WikiaNavigationBarStyle/code.css&action=raw&ctype=text/css'; |
var url7expect = '//dev.wikia.com/index.php?title=MediaWiki:WikiaNavigationBarStyle/code.css&action=raw&ctype=text/css'; | ||
var url7expect2 = '//dev.wikia.com/index.php?title=MediaWiki:WikiaNavigationBarStyle.css&action=raw&ctype=text/css'; | ||
var url8 = 'http://platform.twitter.com/widgets.css'; | ||
var url9 = '//kocka.wikia.com/index.php?title=MediaWiki:UncategorizedFileListing/code.js&action=raw&ctype=text/javascript'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var url9 = '//kocka.wikia.com/index.php?title=MediaWiki:UncategorizedFileListing/code.js&action=raw&ctype=text/javascript'; | |
var url9 = '//kocka.fandom.com/index.php?title=MediaWiki:UncategorizedFileListing/code.js&action=raw&ctype=text/javascript'; |
describe('wikibits', function() { | ||
'use strict'; | ||
mw.config = new mw.Map(); | ||
var url1 = '/index.php?title=MediaWiki:UserTags/code.js&action=raw&ctype=text/javascript'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally I wouldn't store them using names url1, url2 etc..., those names lack context and they should be at least named according to what they store. Going further maybe a good idea would be to set a map like:
{
'UserTagsCodeJsRelative': {
'CodePartRemoved': '....',
'CodePartRemovedWithDomain':'....'
}
}
Using those keys should be easier to read and there would be no need to look for the values.
/*global describe, it, expect*/ | ||
describe('wikibits', function() { | ||
'use strict'; | ||
mw.config = new mw.Map(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that it is reset everywhere like in line :40 , so it's not needed
window.importStylesheetURI = function( url, media ) { | ||
var l = document.createElement( 'link' ); | ||
window.importStylesheetURI = function(url, media) { | ||
var l = document.createElement('link'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
more readable: l -> link
l.type = 'text/css'; | ||
l.rel = 'stylesheet'; | ||
l.href = maybeMakeProtocolRelative(url); | ||
if( media ) { | ||
if (media) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think spaces should be left alone as they were
Per Slack conversation with @Grunny & @KockaAdmiralac. Also prevents hardcoding of $wgScript in importScriptPage function.