Page MenuHomePhabricator

The ext.3d.styles module should not be added unconditionally to every page and should set target mobile
Open, MediumPublic


"The ext.3d.styles module is added to every single desktop page view unconditionally via the onBeforePageDisplay.

It does not get added to mobile as it does not specify a target which is both good (no performance penalty) and bad (no 3d styling capability).

Instead of this, the module could try one of the following strategies:

  1. add itself based on the patterns using the pattern we use in core
  2. Load itself via addModules (if that's possible)
  3. Load the module only only pages where ThreeDThumbnailImage is added to the page

Acceptance criteria

  • Fix the loading mechanism
  • Set targets for the module to mobile.

Event Timeline

Krinkle triaged this task as Medium priority.EditedOct 18 2019, 3:35 AM
Krinkle moved this task from Limbo to Watching on the Performance-Team (Radar) board.
Krinkle added a subscriber: Krinkle.

Will keep an eye on this via our radar. Ping or let us know for any questions or review needs. From my perspective I'd want to know when this was added, why, and what changed since (if anything). In any event, certainly no objection to loading less if all else remains equal in terms of other costs. Sounds good to me!

Once these unknowns are known and removal looks uncontroversial, feel free to add to T233676 as sub task as well.

The 3D viewer doesn't work on mobile yet because it requires MMV.

Change 547763 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/3D@master] Only load 3D styles on pages with 3D images

Change 547763 merged by jenkins-bot:
[mediawiki/extensions/3D@master] Only load 3D styles on pages with 3D images

The 3D viewer doesn't work on mobile yet because it requires MMV.

The module is small, so I think it's okay to set targets to ['mobile', 'desktop'] given this is now restricted to pages with 3d images. It can remind us that we need to fix this to work in mobile and will at least prevent the warnings (which will continue to happen albeit at a much smaller scale).

Keeping this open for remaining acceptance criteria.

Note to self we still need to add the targets to this module.