Last modified: 2013-11-15 15:39:31 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T52746, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 50746 - ResourceLoader: document.write can cause blank page when used from script loaded from mw.loader.using from script in top queue
ResourceLoader: document.write can cause blank page when used from script loa...
Status: NEW
Product: MediaWiki
Classification: Unclassified
ResourceLoader (Other open bugs)
1.22.0
All All
: Normal normal (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks: 53941
  Show dependency treegraph
 
Reported: 2013-07-04 13:35 UTC by Niklas Laxström
Modified: 2013-11-15 15:39 UTC (History)
15 users (show)

See Also:
Web browser: Opera
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Niklas Laxström 2013-07-04 13:35:40 UTC
I get white page at dev.translatewiki.net if I browser there with Opera. It works again if I disable ULS event logging.
Comment 1 Ori Livneh 2013-07-08 07:49:28 UTC
Confirmed on Opera 12 on Windows 7:


[7/8/2013 7:43:58 AM] JavaScript - http://dev.translatewiki.net/
Inline script thread
Uncaught exception: ReferenceError: Undefined variable: mw
Error thrown at line 1, column 0 in http://dev.translatewiki.net/w/load.php?debug=false&lang=en&modules=ext.eventLogging%7Cschema.UniversalLanguageSelector&skin=vector&version=20130708T071740Z&*:
    mw.loader.implement("ext.eventLogging",function(){(function(mw,$,console){'use strict';function ValidationError(message){this.message=message;}ValidationError.prototype=new Error();var self=mw.eventLog={schemas:{},warn:console&&$.isFunction(console.warn)?$.proxy(console.warn,console):mw.log,declareSchema:function(schemaName,meta){if(self.schemas.hasOwnProperty(schemaName)){self.warn('Clobbering existing "'+schemaName+'" schema');}self.schemas[schemaName]=$.extend(true,{revision:-1,schema:{properties:{}},defaults:{}},self.schemas[schemaName],meta);return self.schemas[schemaName];},isInstanceOf:function(value,type){if(value===undefined||value===null){return false;}switch(type){case'string':return typeof value==='string';case'timestamp':return value instanceof Date||(typeof value==='number'&&value>=0&&value%1===0);case'boolean':return typeof value==='boolean';case'integer':return typeof value==='number'&&value%1===0;case'number':return typeof value==='number'&&isFinite(value);default:


Still investigating.
Comment 2 Ori Livneh 2013-07-08 10:28:33 UTC
#wikimedia-dev (very lightly edited):

<MatmaRex> but really, the else clause in core's mediawiki.js, with `document.write( mw.html.element( 'script', { 'src': src }, '' ) );` should be killed
<MatmaRex> i'm pretty sure it doesn't make things any faster on recent browsers
<MatmaRex> which support <script async>
<MatmaRex> and actually i think scripts appended after page starts loading are asynchronous anyway
<MatmaRex> code loads after page content is loaded, but before $.ready() is called; this causes RL to erroneously use document.write for loading it and the page to go blank.
<MatmaRex> this could also be fixed by adding that module to "server-side" dependencies
<MatmaRex> so it would be already loaded by then, and thus RL wouldn't have todo the heavy lifting
<MatmaRex> or by moving everything to bottom queue, which executes after $.ready()
<MatmaRex> or by killing that code path in core
<MatmaRex> i'm really not sure which would be best to do in the long term, and which would be best to just get it working now

(Thanks, MatmaRex, by the way!)

I think the module should be slotted for loading on the server to resolve this particular bug. Not sure about the bigger questions.
Comment 3 Siebrand Mazeland 2013-07-08 10:41:32 UTC
This issue is currently classified as ULS, and the code it is about, is not in production.

Is that correct?
Comment 4 Niklas Laxström 2013-07-08 13:38:58 UTC
I would move this to Mediawiki/ResourceLoader component. We can apply one of the suggested workarounds in ULS.

The code will be in production by tomorrow at the latest, but it is guarded by configuration variable which is not disabled by default.
Comment 5 Niklas Laxström 2013-07-08 13:50:33 UTC
(In reply to comment #4)
> The code will be in production by tomorrow at the latest, but it is guarded
> by
> configuration variable which is not disabled by default.

which *is* disabled by default.
Comment 6 Brad Jorsch 2013-07-08 16:57:11 UTC
@Krinkle: This is the issue from bug 47457 again. The fix we put in place then only avoided the problem for modules in the bottom queue, and now ULS is doing things in the top queue (i.e. ext.uls.init calls mw.loader.using and also has CSS) to trigger it from there.

The basic problem is that document.write cannot be safely called from inside a setTimeout's callback, no matter when in the document it fires. Bug 47872 comment 2 has details. It's also unsafe from <script async>, of course, but modern browsers check for that and raise an error instead of replacing the document. When called synchronously, document.write is safe even at the very end of the document.
Comment 7 Gerrit Notification Bot 2013-07-08 19:27:20 UTC
Change 72614 had a related patch set uploaded by Ori.livneh:
Use 'mw.hook' to queue events until EL has loaded

https://gerrit.wikimedia.org/r/72614
Comment 8 Ori Livneh 2013-07-08 19:29:58 UTC
(In reply to comment #7)
> Change 72614 had a related patch set uploaded by Ori.livneh:
> Use 'mw.hook' to queue events until EL has loaded
> 
> https://gerrit.wikimedia.org/r/72614


The patch above works around the problem in ULS but does not fix ResourceLoader itself.
Comment 9 Gerrit Notification Bot 2013-07-11 12:41:31 UTC
Change 72614 merged by jenkins-bot:
Use $.Callbacks to queue events until EL has loaded

https://gerrit.wikimedia.org/r/72614
Comment 10 Bartosz Dziewoński 2013-07-11 15:17:11 UTC
Per comment 2 I submitted a patch to kill the document.write() code path in core: I897ff585. It works for me, cross-browser tesing and comments very much appreciated.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links