startUp() callback sometimes happen before 'mw' is defined in IE10
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Krinkle
	Oct 24 2017, 7:30 PM

Description

The logic we're using to create a script tag was based on jQuery's script transport:

	script = document.createElement( 'script' );
	script.src = VARS.baseModulesUri;
	script.onload = script.onreadystatechange = function () {
		if ( !script.readyState || /loaded|complete/.test( script.readyState ) ) {
			// Clean up [..]
			// Callback
			startUp();

It seems this is getting dated and I'm not sure why, but I think reducing it to just this would help:

	script = document.createElement( 'script' );
	script.src = VARS.baseModulesUri;
	script.onload = function () {
			// Clean up [..]
			// Callback
			startUp();

Which is what jQuery 3.x does as well.

Details

	Subject	Repo	Branch	Lines +/-
	resourceloader: Remove outdated readyState handler for base modules request	mediawiki/core	wmf/1.31.0-wmf.6	+6 -8
	resourceloader: Remove outdated readyState handler for base modules request	mediawiki/core	master	+6 -8

Customize query in gerrit

Event Timeline

Krinkle created this task.Oct 24 2017, 7:30 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 24 2017, 7:30 PM

Krinkle triaged this task as High priority.Oct 27 2017, 12:31 AM

Krinkle moved this task from Inbox to Confirmed Problem on the MediaWiki-ResourceLoader board.

Krinkle claimed this task.Oct 30 2017, 9:01 PM

Krinkle moved this task from Inbox, needs triage to To-do: Goals prioritized current Quarter on the Performance-Team board.

Krinkle moved this task from To-do: Goals prioritized current Quarter to Doing (old) on the Performance-Team board.

I can't reproduce this on en.wikipedia.org, but I can consistently reproduce it on meta.wikimedia.org.

Screen Shot 2017-10-30 at 19.05.17.png (1×2 px, 906 KB)

After a lot of trial and error I've narrowed it down to a page with the following HTML:

<head>
  <base href="https://meta.wikimedia.org/"/>
  <script async="" src="/w/load.php?debug=false&amp;lang=en&amp;modules=startup&amp;only=scripts&amp;skin=vector"></script>
  <link rel="stylesheet" href="/w/load.php?debug=false&amp;lang=en&amp;modules=site.styles&amp;only=styles&amp;skin=vector"/>
</head>

In addition, it only happens:

In IE 10 (confirmed on Windows 8, and Windows 7). IE11 is unaffected.
When following a link normally. Using the browser Refresh button, the bug does not occur. (Hence, the pen has a self-link to help trigger the bug)
When the page contains an external stylesheet. Without a <link rel=stylesheet> in the <head>, the bug does not occur.

Screen Shot 2017-10-30 at 17.51.45.png (1×1 px, 932 KB)

I can also reproduce it locally, after copying MetaWiki's Common.css to my localhost, and trying it in BrowerStack/IE10 with a local tunnel.

Screen Shot 2017-10-30 at 19.03.52.png (1×1 px, 809 KB)

Applying the following patch helped find which event is firing first.

--- a/resources/src/startup.js
+++ b/resources/src/startup.js
@@ -155,8 +155,19 @@ window.isCompatible = function ( str ) {
 
    script = document.createElement( 'script' );
    script.src = $VARS.baseModulesUri;
-   script.onload = script.onreadystatechange = function () {
+   script.onload = function () {
        if ( !script.readyState || /loaded|complete/.test( script.readyState ) ) {
+           console.log('onload - readyState: ' + script.readyState);
+           // Clean up
+           script.onload = script.onreadystatechange = null;
+           script = null;
+           // Callback
+           startUp();
+       }
+   };
+   script.onreadystatechange = function () {
+       if ( !script.readyState || /loaded|complete/.test( script.readyState ) ) {
+           console.log('onreadystatechange - readyState: ' + script.readyState);
            // Clean up
            script.onload = script.onreadystatechange = null;
            script = null;

In IE10, the callback is consistently fired first by event onreadystatechange - readyState: loaded.

A few patches later

--- a/resources/src/startup.js
+++ b/resources/src/startup.js
@@ -155,13 +155,24 @@ window.isCompatible = function ( str ) {
 
    script = document.createElement( 'script' );
    script.src = $VARS.baseModulesUri;
-   script.onload = script.onreadystatechange = function () {
+   script.onload = function () {
        if ( !script.readyState || /loaded|complete/.test( script.readyState ) ) {
+           console.log('[onload] readyState: ' + script.readyState + '; mw: ' + !!window.mw );
            // Clean up
-           script.onload = script.onreadystatechange = null;
-           script = null;
+           // script.onload = script.onreadystatechange = null;
+           // script = null;
            // Callback
-           startUp();
+           // startUp();
+       }
+   };
+   script.onreadystatechange = function () {
+       if ( !script.readyState || /loaded|complete/.test( script.readyState ) ) {
+           console.log('[onreadystatechange] readyState: ' + script.readyState + '; mw: ' + !!window.mw );
+           // Clean up
+           // script.onload = script.onreadystatechange = null;
+           // script = null;
+           // Callback
+           // startUp();
        }
    };
    document.getElementsByTagName( 'head' )[ 0 ].appendChild( script );

Chrome

[onload] readyState: undefined; mw: true

IE10 fail (regular view)

[onreadystatechange] readyState: loaded; mw: false 
[onload] readyState: complete; mw: true
[onreadystatechange] readyState: complete; mw: true

IE10 pass (reload)

[onload] readyState: complete; mw: true
[onreadystatechange] readyState: complete; mw: true

Change 387509 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resourceloader: Remove outdated readyState handler for base modules request

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

gerritbot added a project: Patch-For-Review.Oct 31 2017, 2:33 AM

Change 387509 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Remove outdated readyState handler for base modules request

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

ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2017-11-07 (1.31.0-wmf.7)).Nov 1 2017, 6:00 PM

Change 387856 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@wmf/1.31.0-wmf.6] resourceloader: Remove outdated readyState handler for base modules request

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

Change 387856 merged by jenkins-bot:
[mediawiki/core@wmf/1.31.0-wmf.6] resourceloader: Remove outdated readyState handler for base modules request

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

Mentioned in SAL (#wikimedia-operations) [2017-11-01T19:57:10Z] <krinkle@tin> Synchronized php-1.31.0-wmf.6/resources/src/startup.js: T178943 (duration: 00m 51s)

ReleaseTaggerBot edited projects, added MW-1.31-release-notes (WMF-deploy-2017-10-31 (1.31.0-wmf.6)); removed MW-1.31-release-notes (WMF-deploy-2017-11-07 (1.31.0-wmf.7)).Nov 1 2017, 8:00 PM

Krinkle closed this task as Resolved.Nov 2 2017, 1:47 AM

Krinkle removed a project: Patch-For-Review.

	F10525610: Screen Shot 2017-10-30 at 19.03.52.png
	Oct 31 2017, 2:16 AM

	F10525676: Screen Shot 2017-10-30 at 19.05.17.png
	Oct 31 2017, 2:05 AM

	F10524711: Screen Shot 2017-10-30 at 17.51.45.png
	Oct 31 2017, 12:52 AM

startUp() callback sometimes happen before 'mw' is defined in IE10Closed, ResolvedPublicActions

Description

Details

Event Timeline

startUp() callback sometimes happen before 'mw' is defined in IE10
Closed, ResolvedPublic
Actions