Page MenuHomePhabricator

ResourceLoader: document.write can cause blank page when used from script loaded from mw.loader.using from script in top queue
Closed, DuplicatePublic

Description

I get white page at dev.translatewiki.net if I browser there with Opera. It works again if I disable ULS event logging.


Version: 1.22.0
Severity: normal

Details

Reference
bz50746

Event Timeline

bzimport raised the priority of this task from to Normal.Nov 22 2014, 1:44 AM
bzimport set Reference to bz50746.
bzimport added a subscriber: Unknown Object (MLST).
ori added a comment.Jul 8 2013, 7:49 AM

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.

ori added a comment.Jul 8 2013, 10:28 AM

#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.

This issue is currently classified as ULS, and the code it is about, is not in production.

Is that correct?

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.

(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.

Anomie added a comment.Jul 8 2013, 4:57 PM

@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.

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

ori added a comment.Jul 8 2013, 7:29 PM

(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.

Change 72614 merged by jenkins-bot:
Use $.Callbacks to queue events until EL has loaded

https://gerrit.wikimedia.org/r/72614

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.