Last modified: 2008-05-19 22:52:51 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 T14773, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 12773 - JS: addOnloadHook() calls after runOnloadHook() fail
JS: addOnloadHook() calls after runOnloadHook() fail
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
Interface (Other open bugs)
unspecified
All All
: Low minor (vote)
: ---
Assigned To: Nobody - You can work on this!
: patch
Depends on:
Blocks: 13232
  Show dependency treegraph
 
Reported: 2008-01-24 12:56 UTC by Conrad Irwin
Modified: 2008-05-19 22:52 UTC (History)
4 users (show)

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


Attachments
A patch for wikibits.js with the strict flag (700 bytes, patch)
2008-04-07 15:53 UTC, Conrad Irwin
Details
A patch for wikibits.js without the strict flag (614 bytes, patch)
2008-04-07 15:53 UTC, Conrad Irwin
Details

Description Conrad Irwin 2008-01-24 12:56:57 UTC
It is possible for addOnloadHook() to be called after runOnloadhooks(). This can happen, for example, when a new script tag is added to the document head using javascript in IE7. This results in the hook not being run, which can cause problems.

Splarka on IRC proposed the following backwards-compatible solution.

function addOnloadHook(hookFunct, strict) {
	// Allows add-on scripts to add onload functions
	if(!doneOnloadHook) {
		onloadFuncts[onloadFuncts.length] = hookFunct;
	} else if(!strict) {
		hookFunct();  // bug in MSIE script loading
	}
}
Comment 1 Random832 2008-01-27 05:00:30 UTC
This also affects firefox. What is the strict flag needed for; I can't think of a situation as a script author where you would want your function to be dropped on the floor due to a race condition.
Comment 2 Conrad Irwin 2008-04-07 12:55:09 UTC
Well, maybe you could educate your users by counting the number of times the script fails to load (with a cookie) and if it fails a few times, alert them to the fact that caching is a good thing (tm). I agree with you, there is not much use for it - however it could (just about plausibly) serve a purpose, and at the expense of 20 bytes may as well stay in there. 
Comment 3 Random832 2008-04-07 15:17:40 UTC
This has nothing to do with caching. The problem is that runOnloadHook is run when the main html page is finished loading, regardless of if other scripts are finished executing or not.

In fact, in some situations caching will just make things _worse_ by allowing runOnloadHook to run _earlier_.
Comment 4 Conrad Irwin 2008-04-07 15:26:33 UTC
It has everything to do with caching, if the javascript files are cached locally the chances are the load times will be small enough that they are loaded and parsed by the time the page finally comes down from the server - if, on the other hand, the javascript has to be loaded as well - then it takes a much longer time to arrive and is much more likely to arrive after addOnloadHook has run. (That is why doing a soft refresh on pages after a hard refresh is sometimes necessary for very dynamically included files).

If you want an example of this happening visit http://en.wiktionary.org/wiki/WT:PREFS , if you haven't been there before the chances are about 50/50 you will see a blank page (The chances seem to be higher in IE7 than FF2 - though I can't convince myself of a good reason for this). Do a soft refresh, or visit another page and come back to that one, and it will magically populate because the load time of the dynamically included script is much smaller when it has been pre downloaded and the addOnloadHook is thus run.
Comment 5 Random832 2008-04-07 15:34:06 UTC
No - the problem - and the reason caching won't help in some cases - is, the javascript has to _execute_ - not just load and parse - before you get to the end of the rendered page.

But my point is, you can't infer from a failure that the user didn't have the js file cached, so it would be inappropriate to use this as a basis to "educate" a user.

Apropos of nothing, I remember some features (like dismissable sitenotice for everyone) getting canned because of the asinine "no cookies for anons" policy. So why is that WT:PREFS page allowed to exist?
Comment 6 Conrad Irwin 2008-04-07 15:53:25 UTC
Created attachment 4791 [details]
A patch for wikibits.js with the strict flag
Comment 7 Conrad Irwin 2008-04-07 15:53:56 UTC
Created attachment 4792 [details]
A patch for wikibits.js without the strict flag
Comment 8 Conrad Irwin 2008-04-07 15:57:34 UTC
I was only proposing that as a possible reason an Extension might want the script flag ;), I agree it's improbable. I have created two patch files, one with the flag, the other without.

(off topic) I don't believe no cookies for anons is a "policy", as anons get session cookies. WT:PREFS is orders of magnitude lower priority than a sitenotice on 'pedia - both because the user base is smaller and also because it is not advertised at the top of every page. If WT:PREFS were causing problems then I'm sure the devs would let us know, and we could then fix it. (We have discussed WT:PREFS a lot on WT:GP and in #wiktionary - so come and chat there if you want to know more about what we think about it)
Comment 9 Random832 2008-04-07 19:58:35 UTC
> The wiki will set a temporary session cookie (PHPSESSID) whenever you
> visit the site. If you do not intend to ever log in, you may deny this
> cookie, but you cannot log in without it. It will be deleted when you
> close your browser session.
>
> More cookies may be set when you log in, to avoid typing in your user
> name (or optionally password) on your next visit. These last up to 30
> days. You may clear these cookies after use if you are using a public
> machine and don't wish to expose your username to future users of the
> machine. (If so, clear the browser cache as well.)

It is implied that users who do not log in will not get any cookies _other than_ PHPSESSID, and I wasn't just making up the fact that this sank a proposed implementation of dismissable sitenotice.
Comment 10 Conrad Irwin 2008-04-07 20:33:17 UTC
(off topic) Please discuss the irrelevant WT:PREFS at http://en.wiktionary.org/wiki/WT:GP or irc://irc.freednode.net#wiktionary - linking to it was intended (like the discussion about caching) to illustrate my point not to sidetrack the fixing of this bug.

The question of having a "strict" flag or not has not been properly resolved, but as it is trivial to include this as a feature we may as well, just in case.
Comment 11 Brion Vibber 2008-05-19 22:52:51 UTC
Applied in r35064 (as part of bug 13232)

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


Navigation
Links