Hi all,
A few days ago I was fiddling around with my Labs instance [1], which serves as a development/testing/showcase area for social tools [2]. Somehow I ended up on Special:ViewPoll [3] and got curious as to why a JavaScript hover effect (mouseover/mouseout) didn't work on that page -- I was sure it used to work just fine not that long time ago. Without thinking that much about it, I clicked on the one and only poll listed on the page [4], and turns out the whole Poll: page is about as broken as it can be due to a JavaScript error.
After a while of debugging, consultation and more debugging, turned out that my local development setup had $wgResourceLoaderDebug = true; in its LocalSettings.php, which apparently hides some race conditions or something like that. The Labs instance tries to be a more faithful representation of a production wiki, and as such, it doesn't have this setting enabled and hence why the problem manifests there.
PollNY itself is a rather old extension, as most original social tools are (see the MW.org page [2] for details), and as such, it likely has some non-optimal code and it's also gone through plenty of iterations in the past. As a matter of fact, when porting PollNY to use ResourceLoader, it seems I myself made some suboptimal choices, such as bundling both CSS and JS into the same module and loading this module with 'position' => 'top'.
Anyway, after decoupling the main CSS into its own module (locally, haven't submitted this to git yet), tweaking the callers and whatnot, I was able to get the hover effects on Special:ViewPoll to work as intended. While this is definitely a step forward, the actual problem with pages in the Poll: namespace still persists.
PollNY has two JS files, LightBox.js and Poll.js. LightBox.js contains a lightbox implementation and technically it's not needed for stuff like the <pollembed> tag etc. and it should only be loaded on Poll: pages. Poll.js, on the other hand, is basically needed everywhere where there is PollNY; special pages, pages that embed a poll via the <pollembed> tag, Poll: pages...
Now, the actual issue is that no matter what I do, I get a "TypeError: 'LightBox' is not defined" on Poll: pages (such as [4]). In the git master version, this is due to the aforementioned race condition: line 466 of Poll.js tries to use mw.loader.load() to load the LightBox RL module if it's not already loaded, but in RL's production mode this fails, because, as I've been told by those with more intimate knowledge of ResourceLoader and its inner workings, mw.loader.load is asynchronous. I've tried mw.loader.using, but it doesn't seem to do anything as far as fixing the issue goes.
Please let me know if you're able to help me out with this; I've ran out of ideas.
[1] http://social-tools.wmflabs.org/ [2] https://www.mediawiki.org/wiki/Social_tools [3] http://social-tools.wmflabs.org/wiki/Special:ViewPoll [4] http://social-tools.wmflabs.org/wiki/Poll:How_is_the_weather_today%3F
Thanks and regards, -- Jack Phoenix MediaWiki developer
How exactly did you try it: mw.loader.using( 'resource.name', function() { ... } ) ?
On Tue, Sep 16, 2014 at 3:00 PM, Jack Phoenix jack@countervandalism.net wrote:
Hi all,
A few days ago I was fiddling around with my Labs instance [1], which serves as a development/testing/showcase area for social tools [2]. Somehow I ended up on Special:ViewPoll [3] and got curious as to why a JavaScript hover effect (mouseover/mouseout) didn't work on that page -- I was sure it used to work just fine not that long time ago. Without thinking that much about it, I clicked on the one and only poll listed on the page [4], and turns out the whole Poll: page is about as broken as it can be due to a JavaScript error.
After a while of debugging, consultation and more debugging, turned out that my local development setup had $wgResourceLoaderDebug = true; in its LocalSettings.php, which apparently hides some race conditions or something like that. The Labs instance tries to be a more faithful representation of a production wiki, and as such, it doesn't have this setting enabled and hence why the problem manifests there.
PollNY itself is a rather old extension, as most original social tools are (see the MW.org page [2] for details), and as such, it likely has some non-optimal code and it's also gone through plenty of iterations in the past. As a matter of fact, when porting PollNY to use ResourceLoader, it seems I myself made some suboptimal choices, such as bundling both CSS and JS into the same module and loading this module with 'position' => 'top'.
Anyway, after decoupling the main CSS into its own module (locally, haven't submitted this to git yet), tweaking the callers and whatnot, I was able to get the hover effects on Special:ViewPoll to work as intended. While this is definitely a step forward, the actual problem with pages in the Poll: namespace still persists.
PollNY has two JS files, LightBox.js and Poll.js. LightBox.js contains a lightbox implementation and technically it's not needed for stuff like the <pollembed> tag etc. and it should only be loaded on Poll: pages. Poll.js, on the other hand, is basically needed everywhere where there is PollNY; special pages, pages that embed a poll via the <pollembed> tag, Poll: pages...
Now, the actual issue is that no matter what I do, I get a "TypeError: 'LightBox' is not defined" on Poll: pages (such as [4]). In the git master version, this is due to the aforementioned race condition: line 466 of Poll.js tries to use mw.loader.load() to load the LightBox RL module if it's not already loaded, but in RL's production mode this fails, because, as I've been told by those with more intimate knowledge of ResourceLoader and its inner workings, mw.loader.load is asynchronous. I've tried mw.loader.using, but it doesn't seem to do anything as far as fixing the issue goes.
Please let me know if you're able to help me out with this; I've ran out of ideas.
[1] http://social-tools.wmflabs.org/ [2] https://www.mediawiki.org/wiki/Social_tools [3] http://social-tools.wmflabs.org/wiki/Special:ViewPoll [4] http://social-tools.wmflabs.org/wiki/Poll:How_is_the_weather_today%3F
Thanks and regards,
Jack Phoenix MediaWiki developer _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On 2014-09-16 3:00 PM, Jack Phoenix wrote:
Hi all,
Now, the actual issue is that no matter what I do, I get a "TypeError: 'LightBox' is not defined" on Poll: pages (such as [4]). In the git master version, this is due to the aforementioned race condition: line 466 of Poll.js tries to use mw.loader.load() to load the LightBox RL module if it's not already loaded, but in RL's production mode this fails, because, as I've been told by those with more intimate knowledge of ResourceLoader and its inner workings, mw.loader.load is asynchronous. I've tried mw.loader.using, but it doesn't seem to do anything as far as fixing the issue goes.
Loading a script that's not in the page is inherently going to be an async thing. You're going to have to write your code to work async.
When you use mw.loader.using to load something in a script always use the callback to wait till the script is actually loaded. It can safely be called when something is already in the page, so just drop the `typeof LightBox == undefined` (this is the wrong way to write a typeof anyways) and do anything with the lightbox in the callback.
mw.loader.using( 'ext.pollNY.lightBox', function() { LightBox.init(); } );
((Any other code using LightBox.* should probably do the same thing, athough mw.loader.using is using jQuery's "promises" (I really want to call this something else, since jQuery doesn't implement proper promises) incorrectly so I'm not sure how well that will work))
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
mw.loader.using( 'ext.pollNY.lightBox', function() { LightBox.init(); } );
((Any other code using LightBox.* should probably do the same thing, athough mw.loader.using is using jQuery's "promises" (I really want to call this something else, since jQuery doesn't implement proper promises) incorrectly so I'm not sure how well that will work))
That's difficult to read with the multiple levels of nesting with parenthesis.
1) jQuery's promises are not compatible with Promises/A+ or ES6 promises spec. ** How is this relevant? 2) mw.loader.using promises incorrectly? ** How? Is there a bug report? What issues does it cause?
Also [1], it is possible to use the regular syntax for jQuery promises: mw.loader.using( 'ext.pollNY.lightBox' ).done( function () { ... } );
[1] Since 1.23 if you dig git history. There is no mention of when it was added in the documentation.
-Niklas
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
On 2014-09-17 12:36 AM, Niklas Laxström wrote:
mw.loader.using( 'ext.pollNY.lightBox', function() { LightBox.init(); } );
((Any other code using LightBox.* should probably do the same thing, athough mw.loader.using is using jQuery's "promises" (I really want to call this something else, since jQuery doesn't implement proper promises) incorrectly so I'm not sure how well that will work))
That's difficult to read with the multiple levels of nesting with parenthesis.
- jQuery's promises are not compatible with Promises/A+ or ES6 promises spec.
** How is this relevant?
They implement barely 1/3 of the fundamentals of what a promise is, missing out on nearly everything that makes promises a powerful tool, so I just loath the idea of EVER using the term promise to refer to them.
- mw.loader.using promises incorrectly?
** How? Is there a bug report? What issues does it cause?
Sorry, it looks like I'm probably wrong about this.
I looked up mw.loader.using's code when I was writing the example, but I didn't see the complex handling in request, filter, and mw.loader.work.
So when I saw the pattern in mw.loader.using that went... 1. create brand new deferred 2. resolve if all deps are ready 3. reject if any are missing 4. otherwise call request()
...It looked like there might be a race condition where calling mw.loader.using again before the script loading was complete would cause a second HTTP request to be triggered instead of waiting for the first to complete.
On Wed, Sep 17, 2014 at 1:55 AM, Daniel Friesen daniel@nadir-seen-fire.com wrote:
mw.loader.using( 'ext.pollNY.lightBox', function() { LightBox.init(); } );
Yep, this is exactly what another fellow developer suggested to me earlier on and I tried it only to find out that it doesn't appear to be working either. :-(
Thanks and regards, -- Jack Phoenix MediaWiki developer
It should, if it's still broken there should be some other issue.
What errors do you get, before and aster.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
On 2014-09-17 7:37 AM, Jack Phoenix wrote:
On Wed, Sep 17, 2014 at 1:55 AM, Daniel Friesen daniel@nadir-seen-fire.com wrote:
mw.loader.using( 'ext.pollNY.lightBox', function() { LightBox.init(); } );
Yep, this is exactly what another fellow developer suggested to me earlier on and I tried it only to find out that it doesn't appear to be working either. :-(
Thanks and regards,
Jack Phoenix MediaWiki developer _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On Wed, Sep 17, 2014 at 10:05 PM, Daniel Friesen <daniel@nadir-seen-fire.com
wrote:
It should, if it's still broken there should be some other issue.
What errors do you get, before and aster.
Same old "TypeError: LightBox is not defined". At first I was thinking of cache pollution, but that doesn't seem to be the problem here, since I've purged my caches and the problem just seems to persist. I've wrapped all three usages (onload handler, Poll.loadingLightBox() and Poll.goToNewPoll()'s .done() handler) around the mw.loader.using call you suggested, but to no avail. As you'd guess, it still magically works with debug=true in the URL.
Thanks and regards, -- Jack Phoenix MediaWiki developer
wikitech-l@lists.wikimedia.org