Last modified: 2014-02-12 23:52:54 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 T46070, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 44070 - Dynamically loading SpecialPage ResourceLoader modules causes unnecessary processing on ALL requests
Dynamically loading SpecialPage ResourceLoader modules causes unnecessary pro...
Status: RESOLVED FIXED
Product: MobileFrontend
Classification: Unclassified
stable (Other open bugs)
unspecified
All All
: Unprioritized normal
: ---
Assigned To: Jon
:
Depends on: 41340 44072
Blocks:
  Show dependency treegraph
 
Reported: 2013-01-17 18:47 UTC by Arthur Richards
Modified: 2014-02-12 23:52 UTC (History)
12 users (show)

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


Attachments

Description Arthur Richards 2013-01-17 18:47:52 UTC
Changesets https://gerrit.wikimedia.org/r/#/c/43979/ and https://gerrit.wikimedia.org/r/#/c/44442/ introduce changes that make mobile ResourceLoader modules for SpecialPages get dynamically defined. This is neat, however the current implementation in master (https://gerrit.wikimedia.org/r/#/c/44442/) makes it so that the modules get defined on EVERY request, including API requests, which adds unnecessary overhead.

We initially attempted to have the processing happen ONLY inside of the invocation of the ResourceLoaderRegisterModules hook, which works in MobileFrontend alpha/beta, but not in production. There are a couple of reasons for this:
1) The ResourceLoaderRegisterModules hook runs AFTER SkinMobile::attachAdditionalPageResources() attaches resources (when RL is not fully in use, eg in production). You can try to get around that by forcing the hook to run by calling OutputPage::getResourceLoader() in SkinMobile::attachAdditionalPageResources(), but that bumps into another problem:
2) using $resourceLoader->registerModule() in an invocation of the ResourceLoaderRegisterModules hook appears to not work properly for modules that need to be loaded at the bottom of the page. I'll be filing a separate bug for that.

We should get this fixed sooner rather than later. A few options I can think of:
* Fix #2 above
* Add a hook in SkinMobile to dynamically load last-minute modules into $wgResourceModules (this is ugly, but would go away once we resolve https://bugzilla.wikimedia.org/show_bug.cgi?id=41340)
Comment 1 Arthur Richards 2013-01-17 19:09:08 UTC
Actually it sounds like the issue I mentioned in #2 above is NOTE a ResourceLoader issue, but rather a problem in MobileFrontend: https://bugzilla.wikimedia.org/show_bug.cgi?id=44072
Comment 2 Jon 2013-02-09 00:48:22 UTC
Is this fixed now?
Comment 3 Arthur Richards 2013-02-11 18:51:31 UTC
No, the SpecialPage related modules are still getting prepared on every request. They should get handled in MobileFrontendHooks::onResourceLoaderRegisterModules(). 

Since bug 44072 is fixed now, this should be totally doable.
Comment 4 Jon 2013-03-05 17:57:57 UTC
I've seen this code is proving to cause troubles for people understanding our code, so I put my hand up and say this was a bad idea. Instead I will manually define the modules. Patch on it's way.
Comment 5 Krinkle 2013-03-05 18:20:30 UTC
I don't know any context behind this bug, but I just want to make a quick point.

Remember that load.php/startup module needs it, since they are loaded not from within the page.

Module definitions mustn't rely on page context. The startup module needs a complete overview (which is by design).
Comment 6 Jon 2013-03-05 18:40:43 UTC
https://gerrit.wikimedia.org/r/52254

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


Navigation
Links