610369 - Wikia.com editor broken: load order for scripts inserted as DOM nodes no longer matches the order of insertion

Status: RESOLVED FIXED

(Tech Evangelism Graveyard :: English US, defect)

Description

[Wladimir Palant]

Attachment: Minimal content policy for testing

Opening http://runescape.wikia.com/index.php?title=Zamorak&action=edit#EditPage usually displays an editor. This doesn't happen in Minefield (build 20101105 on Windows 7 x64) if an extension implementing a content policy is present - e.g. Adblock Plus or the minimal content policy (attached). Error Console displays a bunch of errors about functions not being defined (addButton, $, hookEvent, jQuery), e.g:

Error: addButton is not defined
Source File: http://runescape.wikia.com/index.php?title=Zamorak&action=edit#EditPage
Line: 557

Switching javascript.options.tracejit.content to false "fixes" this, javascript.options.methodjit.content on the other hand doesn't seem to have an effect.

When I tried to find a regression range I noticed that opening that page at startup will sometimes succeed. The definitive regression range is 2010-09-17 to 2010-09-18:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=268ef4ccb5ff&tochange=fc8d8dfee20f

Comment 1

[Wladimir Palant]

All the variables in question are defined after the page loads - so the problem isn't that the script files didn't load, it's rather the order in which they loaded. The code loading the scripts is this:

> var len = jsReferences.length;
> for(var i=0; i<len; i++)
>   wsl.loadScript(jsReferences[i]);

With wsl.loadScript() being:

> function(b,c){
>   this.useDOMInjection?
>     this.loadScriptDOMInjection(b,c):
>     this.loadScriptDocumentWrite(b,c)
> }

useDOMInjection is set by some wacky user-agent sniffing. The issue only occurs if this variable is set to true. For sake of completeness, function wsl.loadScriptDOMInjection():

> function(b,c){
>   var a=document.createElement("script");
>   a.type="text/javascript";
>   a.src=b;
>   var d=function(){
>     a.onloadDone=true;
>     typeof c=="function"&&c()
>   };
>   a.onloadDone=false;
>   a.onload=d;
>   a.onreadystatechange=function(){
>     a.readyState=="loaded"&&!a.onloadDone&&d()
>   };
>   this.headNode.appendChild(a)
> }

Comment 2

[Wladimir Palant]

Confirmed with a modified local copy of the page that would dump the script URLs as they load. The problem is the order in which scripts load if they are inserted as DOM nodes. In Firefox 3.6.12 the load order matches the order of insertion:

> http://runescape.wikia.com/static/js/oasis_anon_everything_else_js/2e48169e137a6bec84b572406af9cf89.js
> http://images.wikia.com/common/__cb29542/skins/common/edit.js?29542
> http://images.wikia.com/common/__cb29542/extensions/wikia/RTE/ckeditor/ckeditor.js?29542
> http://images.wikia.com/common/__cb29542/skins/common/jquery/jquery-ui-1.7.2.custom.js?29542
> http://images.wikia.com/common/__cb29542/skins/common/jquery/jquery.json-1.3.js?29542
> http://images.wikia.com/common/__cb29542/extensions/wikia/WikiaMiniUpload/js/WMU.js?29542
> http://images.wikia.com/common/__cb29542/extensions/wikia/WikiaPhotoGallery/js/WikiaPhotoGallery.js?29542
> http://images.wikia.com/common/__cb29542/extensions/wikia/CategorySelect/CategorySelect.js?29542
> http://images.wikia.com/common/__cb29542/extensions/wikia/AdSS/adss.js?29542
> http://images.wikia.com/common/__cb29542/extensions/wikia/EditEnhancements/js/EditEnhancements.js?29542
> /index.php?title=-&action=raw&gen=js&useskin=oasis

In Minefield the order is "random", I guess that all scripts load in parallel. Here an example:

> http://runescape.wikia.com/static/js/oasis_anon_everything_else_js/2e48169e137a6bec84b572406af9cf89.js
> http://images.wikia.com/common/__cb29542/skins/common/edit.js?29542
> http://images.wikia.com/common/__cb29542/extensions/wikia/RTE/ckeditor/ckeditor.js?29542
> http://images.wikia.com/common/__cb29542/extensions/wikia/CategorySelect/CategorySelect.js?29542
> http://images.wikia.com/common/__cb29542/skins/common/jquery/jquery-ui-1.7.2.custom.js?29542
> http://images.wikia.com/common/__cb29542/skins/common/jquery/jquery.json-1.3.js?29542
> http://images.wikia.com/common/__cb29542/extensions/wikia/WikiaMiniUpload/js/WMU.js?29542
> http://images.wikia.com/common/__cb29542/extensions/wikia/WikiaPhotoGallery/js/WikiaPhotoGallery.js?29542
> /index.php?title=-&action=raw&gen=js&useskin=oasis
> http://images.wikia.com/common/__cb29542/extensions/wikia/AdSS/adss.js?29542
> http://images.wikia.com/common/__cb29542/extensions/wikia/EditEnhancements/js/EditEnhancements.js?29542

Not sure what this has to do with content policies or tracing...

Comment 3

[Wladimir Palant]

Attachment: Minimized testcase

Created a minimal testcase - this will insert four script tags into the document and record the order in which these scripts load. In Firefox 3.6.12 the result is always the same:

https://bugzilla.mozilla.org/js/util.js
https://bugzilla.mozilla.org/js/global.js
https://bugzilla.mozilla.org/js/flag.js
https://bugzilla.mozilla.org/js/field.js

This matches the order in which the scripts have been inserted. In Minefield however the scripts often load in a different order. And, contrary to what I said above, this issue is still reproducible both without content policies and with tracing disabled - apparently these only change the timing but are otherwise not related to the problem.

Comment 4

[Wladimir Palant]

Ok, this is apparently caused by bug 591981. Jonas, given that it breaks wikia.com - what should happen to this bug? Tech Evangelism? Note that Wikia.com is explicitly checking for Opera and Firefox, in other browsers it uses document.write().

Comment 5

[Henri Sivonen (:hsivonen)]

I think this should be evang at this point in time. Certainly not JS Engine.

It would certainly be very sad if we failed at evang here and had to treat this as a DOM bug. Note that the patches in bug 602838 will give sites a way to opt into ordered behavior.

Comment 6

[Henri Sivonen (:hsivonen)]

That is, it would be super-nice to land bug 602838 before contacting the site in order to have a solution ready.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Let's just add a dep; that will also remind us to contact them once we have said solution.

Comment 8

[Henri Sivonen (:hsivonen)]

I sent email to Wikia.

Comment 9

[Henri Sivonen (:hsivonen)]

I got a reply from Wikia. They'll fix this once Firefox 4.0 final has shipped.

Comment 11

[Henri Sivonen (:hsivonen)]

(In reply to comment #9)
> I got a reply from Wikia. They'll fix this once Firefox 4.0 final has shipped.

In case someone else wants to consolidate their pending Wikia support requests with this one, this one is Wikia Support Request #91538.

Comment 12

[Henri Sivonen (:hsivonen)]

I reminded Wikia of this problem now that we have release candidate candidates. 

While doing so, I noticed that Wikia had added sniffing for "/4.0b" to work around this. Obviously, that workaround will break when RC is pushed to the beta update channel. Something to be aware about if we start getting bug report about Wikia at that point.

Comment 13

[Boris Zbarsky [:bzbarsky]]

That will also break in other Gecko-based browsers.  Can we get them to at least sniff correctly if they insist on sniffing?  :(

Comment 14

[Henri Sivonen (:hsivonen)]

(In reply to comment #13)
> That will also break in other Gecko-based browsers.  Can we get them to at
> least sniff correctly if they insist on sniffing?  :(

I suggested for sniffing for "rv:1.9." and running the HTML5-incompatible code if that's found if they want to run different code for old versions. I also pointed out that their current sniffing is going to break when Opera implements the spec.

Comment 15

[Henri Sivonen (:hsivonen)]

I got a reply that they'll fix once Firefox 4 final is available for download from getfirefox.com.

Comment 16

[Henri Sivonen (:hsivonen)]

Wikia has fixed this for Firefox, so marking FIXED. this.useDOMInjection=b.indexOf("opera")!=-1||b.indexOf("firefox/3.")!=-1;

It will still break when Opera complies with the spec. :-(

Comment 17

[Chris Lawson (gone)]

(In reply to comment #16)
> Wikia has fixed this for Firefox, so marking FIXED.
> this.useDOMInjection=b.indexOf("opera")!=-1||b.indexOf("firefox/3.")!=-1;

That doesn't sound like it's fixed for any non-Firefox Gecko browsers (at least, without the word "Firefox" followed by a slash and a version number in the UA string), though.

I'm going to reopen this pending the implementation of comment 14, or a valid explanation from Wikia as to why they can't do that.

Comment 18

[Boris Zbarsky [:bzbarsky]]

It's fixed for any non-Firefox browser based on Gecko 2.0.

And I suspect it's fixed in all of them, if the "not opera or firefox" codepath works in old Geckos, which it usually does in cases like this.  Have you actually tested a non-Firefox browser here and run into problems?

Comment 19

[Chris Lawson (gone)]

I don't understand how the code in comment 16 is in any way "right", but I don't have a good way to test Wikia in Camino since the steps in comment 0 aren't valid in Camino in the first place.

cl

Comment 20

[Boris Zbarsky [:bzbarsky]]

That code is testing when Wikia should use broken code.  In all other cases it uses working code that works fine in Gecko, as far as I can tell.

I don't know _why_ they insist on having a codepath on which they use broken code, but they do.

Comment 21

[Henri Sivonen (:hsivonen)]

(In reply to comment #19)
> I don't understand how the code in comment 16 is in any way "right", 

It isn't "right", but it the bogosity is harmless now for Gecko-based browsers and, unfortunately, the rest is Opera's evangelism problem now that I've twice failed to get Wikia to be nice to Opera, too.

Resolving again due to lack of evidence of breakage in non-Firefox Gecko-based browsers.

"firefox/3." won't match in browsers based on Gecko 1.x that don't have a Firefox token. It will match but the match will be harmless in Gecko 1.x-based browsers that do have a Firefox token. It won't match in Gecko 2.x-based browsers.