Last modified: 2014-04-06 13:25:29 UTC
On wiki's with Scribunto installed, when viewed in the mobile site, the exception "Uncaught Error: Unknown dependency: ext.scribunto" is thrown which prevents any further JavaScript from running. This is because the module is not registered on mobile. It also depends on jquery.ui.dialog code which pulls in the entire jquery.ui library - we certainly do not want on mobile where JavaScript load should be as tiny as possible. I doubt we'd want to run this code on mobile thus the use of addInlineScript needs to be avoided since the module doesn't exist in the mobile environment. At the very least it should be wrapped with a try catch so say it is acceptable to fail. Seems to have been introduced by https://gerrit.wikimedia.org/r/#/c/5617/1,publish
Okay this doesn't look so bad now as it seems to only become a problem when a ScribuntoError occurs (I'm not sure how often this occurs on wiki's - I've only managed to replicate this locally). However it seems bizarre to me that the entire jQuery UI library would be loaded simply for error reporting...
Realistically, Scribunto does not require the full jQuery UI. I feel that it should roll its own "script error" popup, since jQuery UI Dialogs were not really designed for this use.
What's the point of having jQuery if we're not supposed to use it?
For desktop, jQuery UI is fine (at least until OO.js is more widely adopted). The only problem is that mobile doesn't want to load jQuery UI unless absolutely necessary, and Scribunto is used on both desktop and mobile. There are two possible short-term solutions: * Make jquery.ui and jquery.ui.dialog loadable on mobile by giving them the 'mobile' target in the module definition (and make sure they are only loaded when there is an error) * Remove the dialog code from Scribunto and just use a JS alert in the meantime. Alerts are pretty unfriendly (since they take over the browser, stop JS execution, etc.), but since it's such a limited use, it might be OK for a short-term fix.
jQuery UI != jQuery Use jQuery by all means of a jQuery plugin for this - you do not need a huge library the size of jQuery UI. jQuery UI is totally unsuitable for mobile. Please do not load it to simply use I'd guess about 2% of the library.. As stated the correct thing to do would be to start using OO.js which is in core.
In case I was clear please do not follow Ryan's suggestion of "Make jquery.ui and jquery.ui.dialog loadable on mobile by giving them the 'mobile' target in the module definition (and make sure they are only loaded when there is an error)" An alert would make more sense, using jQuery UI is overkill for this use case imo.
(In reply to comment #4) > * Remove the dialog code from Scribunto and just use a JS alert in the > meantime. Alerts are pretty unfriendly (since they take over the browser, > stop > JS execution, etc.), but since it's such a limited use, it might be OK for a > short-term fix. I believe some browsers are starting to block alters entirely. (In reply to comment #5) > As stated the correct thing to do would be to start using OO.js which is in > core. Patches welcome. Or even links to examples.
OOjs is in core, but if you want to make dialogs, you probably need OOjs UI as well, which is still only in VisualEditor.
One problem with alert() is that it's not HTML, so you couldn't click on the source file links which appear in the Scribunto backtraces. (In reply to comment #1) > Okay this doesn't look so bad now as it seems to only become a problem when a > ScribuntoError occurs (I'm not sure how often this occurs on wiki's - I've > only managed to replicate this locally). You can replicate it on the pages in this category: https://en.wikipedia.org/wiki/Category:Pages_with_script_errors It looks like there are currently no such pages in the main namespace, so the impact of this bug on readers of en.m.wikipedia.org is negligible. There's a bug in Scribunto which causes jquery.ui.dialog and the hidden category to be added even if no errors are emitted into the final HTML, say due to the error being sent to the condition of a {{#iferror:}}. At one point, there were tens of thousands of pages in [[Category:Pages with script errors]] for no obvious reason, possibly due to that bug. So, it would have affected mobile readers at that time. That could be reported separately if it's not already. > However it seems bizarre to me that the > entire jQuery UI library would be loaded simply for error reporting... I think error reporting is the most important feature in Scribunto, it's central to the usability of Lua scripting. So I don't think it's bizarre that it loads libraries. It's just somewhat inefficient. I would be fine with a lightweight DIY thing, or with dynamic loading of jquery.ui.dialog on click of the error message, with a spinner.
let's just dynamically load it, it's simple and not too many 'normal' users are using it anyways. When OOjs is more mature, we can just switch it over.
Updated summary since of course it doesn't pull in the whole of jQuery UI, it pulls in jquery.ui.dialog and (implicitly) its 7 dependencies, so 8 out of the 17 JavaScript files in resources/jquery.ui/
Change 109262 had a related patch set uploaded by TheDJ: Conditionally load jquery.ui.dialog for Scribuntu errors https://gerrit.wikimedia.org/r/109262
Change 118944 had a related patch set uploaded by Hoo man: Don't try to (inline load) ext.scribunto if it isn't registered https://gerrit.wikimedia.org/r/118944
Change 118944 merged by jenkins-bot: Don't try to inline load ext.scribunto on mobile https://gerrit.wikimedia.org/r/118944
I'd still advise you to move away from jquery.ui but this is not effecting mobile now so I am happy.
Change 109262 abandoned by TheDJ: Conditionally load jquery.ui.dialog for Scribunto errors Reason: Fixed with: https://gerrit.wikimedia.org/r/#/c/118944/ https://gerrit.wikimedia.org/r/109262
