r44192 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r44191‎ | r44192 | r44193 >
Date:21:52, 3 December 2008
Author:brion
Status:ok (Comments)
Tags:
Comment:
* (bug 13342) importScript() generates more consistent URI encoding

Switches order of URL to put title first, and encodes ':' and '/' consistently with MediaWiki general URI encoding.


The current implementation escapes ':' and '/', producing URLs like this:

/trunk/index.php?action=raw&ctype=text/javascript&title=MediaWiki%3ASysop.js

If we're recommending consistent use of importScript() it probably doesn't really matter, but it wouldn't hurt to be self-consistent with how we generate other URLs... of course even with the change to the encoding, the order of the pieces is different.

MediaWiki usually generates URLs with the title component first; also caching options will differ depending on who's generating the URL and when.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/skins/common/wikibits.js (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/common/wikibits.js
——@@ -48,7 +48,10 @@
4949 }
5050
5151 function importScript(page) {
52 - return importScriptURI(wgScript + '?action=raw&ctype=text/javascript&title=' + encodeURIComponent(page.replace(/ /g,'_')));
 52+ var uri = wgScript + '?title=' +
 53+ encodeURIComponent(page.replace(/ /g,'_')).replace('%2F','/').replace('%3A',':') +
 54+ '&action=raw&ctype=text/javascript';
 55+ return importScriptURI(uri);
5356 }
5457
5558 var loadedScripts = {}; // included-scripts tracker
Index: trunk/phase3/RELEASE-NOTES
——@@ -392,6 +392,7 @@
393393 pages if the repo wiki had a different canonical name for the File: namespace.
394394 Added 'fileNamespace' configuration item to $wgForeignFileRepos to override
395395 the local canonical name 'File' with another string.
 396+* (bug 13342) importScript() generates more consistent URI encoding
396397
397398
398399 === API changes in 1.14 ===

Comments

#Comment by Splarka (talk | contribs)   01:57, 4 December 2008

I am told you shouldn't do more than one .replace() per assignment, as apparently some older parsers will fail all of them if one finds no matches.

#Comment by Brion VIBBER (talk | contribs)   20:11, 10 December 2008

If it finds no matches it just won't replace anything... otherwise something's *seriously* broken! :)

Status & tagging log