r96679 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r96678‎ | r96679 | r96680 >
Date:18:39, 9 September 2011
Author:brion
Status:resolved (Comments)
Tags:
Comment:
MFT r87986 -- consolidation of addScript in mediawiki.loader, also fixes bug 30825 by switching to explicit DOM addChild for script node, bypassing weird behavior in jQuery that caused script nodes to be loaded via AJAX, which fails for protocol-relative URLs.
Modified paths:
  • /branches/wmf/1.17wmf1/resources/mediawiki/mediawiki.js (modified) (history)

Diff [purge]

Index: branches/wmf/1.17wmf1/resources/mediawiki/mediawiki.js
@@ -770,6 +770,26 @@
771771 return sorted;
772772 }
773773
 774+ /**
 775+ * Adds a script tag to the body, either using document.write or low-level DOM manipulation,
 776+ * depending on whether document-ready has occured yet.
 777+ */
 778+ function addScript( src ) {
 779+ if ( ready ) {
 780+ // jQuery's getScript method is NOT better than doing this the old-fassioned way
 781+ // because jQuery will eval the script's code, and errors will not have sane
 782+ // line numbers.
 783+ var script = document.createElement( 'script' );
 784+ script.setAttribute( 'src', src );
 785+ script.setAttribute( 'type', 'text/javascript' );
 786+ document.body.appendChild( script );
 787+ } else {
 788+ document.write( mw.html.element(
 789+ 'script', { 'type': 'text/javascript', 'src': src }, ''
 790+ ) );
 791+ }
 792+ }
 793+
774794 /* Public Methods */
775795
776796 /**
@@ -834,22 +854,12 @@
835855 batch = [];
836856 // Asynchronously append a script tag to the end of the body
837857 function request() {
838 - var html = '';
839858 for ( var r = 0; r < requests.length; r++ ) {
840859 requests[r] = sortQuery( requests[r] );
841 - // Build out the HTML
842860 var src = mediaWiki.config.get( 'wgLoadScript' ) + '?' + $.param( requests[r] );
843 - html += mediaWiki.html.element( 'script',
844 - { type: 'text/javascript', src: src }, '' );
 861+ addScript( src );
845862 }
846 - return html;
847863 }
848 - // Load asynchronously after doumument ready
849 - if ( ready ) {
850 - setTimeout( function() { $( 'body' ).append( request() ); }, 0 )
851 - } else {
852 - document.write( request() );
853 - }
854864 }
855865 };
856866
@@ -1011,13 +1021,7 @@
10121022 .attr( 'href', modules ) );
10131023 return true;
10141024 } else if ( type === 'text/javascript' || typeof type === 'undefined' ) {
1015 - var script = mediaWiki.html.element( 'script',
1016 - { type: 'text/javascript', src: modules }, '' );
1017 - if ( ready ) {
1018 - $( 'body' ).append( script );
1019 - } else {
1020 - document.write( script );
1021 - }
 1025+ addScript( modules );
10221026 return true;
10231027 }
10241028 // Unknown type
@@ -1177,7 +1181,6 @@
11781182 };
11791183 } )();
11801184
1181 -
11821185 /* Extension points */
11831186
11841187 this.legacy = {};

Follow-up revisions

RevisionCommit summaryAuthorDate
r96680Followup r87986: qunit test case for bug 30825...brion18:43, 9 September 2011
r96682Followup r96679 -- fix for dumb mistake in merge...brion19:01, 9 September 2011
r96699Provisional workaround for bug 30825 as it affects things run via $.ajax() us...brion22:03, 9 September 2011
r97404Move the mediawiki.test.bug30825.js helper script out of the /suites/ directo...krinkle04:50, 18 September 2011
r101356fix mediawiki.test wrong URL...hashar13:53, 31 October 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r87986(reverts r87985, and does what that was SUPPOSED to do!) Gerenalized code use...tparscal15:22, 13 May 2011

Comments

#Comment by Reedy (talk | contribs)   18:58, 9 September 2011

Fixme per #mediawiki

#Comment by Brion VIBBER (talk | contribs)   19:03, 9 September 2011

I fudged up one part of the merge by forgetting to hoist code out of a function that has been removed, causing some modules to not get loaded at all.

Unfortunately we accidentally deployed it before discovering this (d'oh!); Reedy's got it reverted for now but some users did encounter the breakage.

r96682 should fix it.

Status & tagging log