Page MenuHomePhabricator

Updating from addOnloadHook to jQuery(document).ready breaks some scripts (tracking)
Closed, InvalidPublic

Description

According to [[mw:ResourceLoader/JavaScript Deprecations#wikibits.js]], the function "addOnloadHook" is deprecated in favour of "jQuery(document).ready".

This change breaks some scripts.[1][2]

E.g.: consider a function to get the rows of the table present on

[[Special:Upload]]:

function test(){

document.getElementById( 'mw-htmlform-description' ).rows

}

If it is executed by "addOnloadHook", it will return all the 6 rows, but if it is executed by "jQuery(document).ready", it only returns 4 rows.

You can confirm this broken behavior using the sample code [3].

[1] https://pt.wikipedia.org/w/index.php?oldid=27372605
[2] https://pt.wikipedia.org/wiki/MediaWiki_Discuss%C3%A3o:Gadget-UploadForm.js#Gadget_padr.C3.A3o_e_ResourceLoader
[3] https://en.wikipedia.org/?oldid=457179751


Version: unspecified
Severity: normal

Details

Reference
bz31926

Event Timeline

bzimport raised the priority of this task from to Normal.Nov 21 2014, 11:53 PM
bzimport added a project: MediaWiki-JavaScript.
bzimport set Reference to bz31926.
bzimport added a subscriber: Unknown Object (MLST).
He7d3r created this task.Oct 24 2011, 5:53 PM
brion added a comment.Oct 24 2011, 6:03 PM

Those 2 other rows are added by other JavaScript; presumably you're managing to get loaded & run before it does.

What are you trying to do with them, and is initialization time the right time to do it?

(In reply to comment #1)

Those 2 other rows are added by other JavaScript; presumably you're managing to
get loaded & run before it does.

These extra rows are also present on en.wp, pt.wp and my local copy of MediaWiki.

I noticed the problem on Portuguese Wikipedia after this change
https://pt.wikipedia.org/w/index.php?diff=27372605&oldid=27372578
which replaced (among other things) "addOnloadHook" by "$".

brion added a comment.Oct 24 2011, 6:14 PM

Yes, they're added by JavaScript that ships with MediaWiki, in skins/common/upload.js. So this is expected to see it on multiple sites. :)

This is older code and still gets run via addOnloadHook(), which I assume runs somewhere during or after the DOMReady event stack.

(In reply to comment #3)

Yes, they're added by JavaScript that ships with MediaWiki, in
skins/common/upload.js. So this is expected to see it on multiple sites. :)

Ah, ok. :-)

This is older code and still gets run via addOnloadHook(), which I assume runs
somewhere during or after the DOMReady event stack.

Should I open another bug requesting it to be updated to ResourceLoader?

(In reply to comment #4)

Should I open another bug requesting it to be updated to ResourceLoader?

Yes, please. And maybe close this bug unless it addresses a more general concern. Or just change the summary for this bug to point to the particular issue?

(In reply to comment #5)

(In reply to comment #4)

Should I open another bug requesting it to be updated to ResourceLoader?

Yes, please.

Done: Bug 31946.

And maybe close this bug unless it addresses a more general
concern. Or just change the summary for this bug to point to the particular
issue?

I've seem this this kind of problem happening on Wikimedia Commons, with user made portlets (since bug 23515 is still open). See the comment on this diff:
http://commons.wikimedia.org/w/index.php?title=MediaWiki%3AInterProject.js&action=historysubmit&diff=45407092&oldid=45400093

But that problem seems to be fixed, since they are using jQuery(document).ready again:
http://commons.wikimedia.org/w/index.php?title=MediaWiki%3AInterProject.js&action=historysubmit&diff=49978594&oldid=45699279

This happens because both jQuery and addOnloadHook have their own queue. JavaScript is single threaded and both are executing in the original order. So two functions that both call addOnloadHook(), the first one will execute first, and the second will interface the document the first left behind.

If a function depends on another function adding rows, then scheduling in addOnloadHook or $(document).ready is not sufficient, it should instead add a dependency to the other script and/or use a hook or other type of callback.

Marking this bug as invalid, the differences between addOnloadHook $(document).ready mentioned here are not differences at all. They both work exactly the same.

The reason things fail is because code is apparently relying on something it shouldn't be relying on.

If 2 scripts both schedule in addOnloadHook, and one relies on it that the other is in the queue first, that's a problem. It is bound to fail one way or another and now it has.

The simplest solution is to just use $(document).ready everywhere. A more long-term solution is to make sure not to depend on execution order, javascript is too good for that :)

Aklapper added a subscriber: Aklapper.

[adding the Tracking-Neverending project to tasks blocking (now deprecated) T4007 as part of T93366]