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

Assigned To

Authored By

	Jdlrobson
	Oct 17 2019, 7:14 PM

Description

"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:

add itself based on the patterns using the pattern we use in core
Load itself via addModules (if that's possible)
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.

Details

	Subject	Repo	Branch	Lines +/-
	Only load 3D styles on pages with 3D images	mediawiki/extensions/3D	master	+25 -3

Customize query in gerrit

Related Objects
Search...

Status	Assigned	Task
Open	None	T158181 Aim for workflow equivalence for MediaWiki on desktop and mobile web
Resolved	Jdlrobson	T127268 Dismantle ResourceLoader's "targets" system
Resolved	Jdlrobson	T235712 Fix the most common "Module not loadable on target mobile" warnings (Oct 2019)
Resolved	Jdlrobson	T235796 The ext.3d.styles module should not be added unconditionally to every page and should set target mobile

Event Timeline

Jdlrobson created this task.Oct 17 2019, 7:14 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 17 2019, 7:14 PM

Jdlrobson mentioned this in T235797: [SPIKE, 16hrs] We should not need 164kb of JS and 4.89kb of CSS (gzipped) to render a blank page in safemode.Oct 17 2019, 7:14 PM

Masumrezarock100 subscribed.Oct 17 2019, 7:29 PM

Krinkle moved this task from Inbox, needs triage to Radar on the Performance-Team board.Oct 18 2019, 3:32 AM

Krinkle edited projects, added Performance-Team (Radar); removed Performance-Team.

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.

Jdlrobson added a parent task: T235712: Fix the most common "Module not loadable on target mobile" warnings (Oct 2019).Oct 28 2019, 8:44 PM

Jdlrobson mentioned this in T235712: Fix the most common "Module not loadable on target mobile" warnings (Oct 2019).Oct 30 2019, 7:05 PM

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

https://gerrit.wikimedia.org/r/547763

gerritbot added a project: Patch-For-Review.Nov 1 2019, 6:14 PM

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

https://gerrit.wikimedia.org/r/547763

ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.5; 2019-11-05).Nov 1 2019, 11:00 PM

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.

Jdlrobson moved this task from Untriaged to Untag on the Web-Team-Backlog (Tracking) board.Sep 21 2021, 8:48 PM

LGoto removed a project: Web-Team-Backlog (Tracking).Nov 9 2021, 4:23 PM

This code is now loaded on mobile. It was fixed by https://gerrit.wikimedia.org/r/885788

From the task description:

[…] which is both good (no performance penalty) and bad (no 3d styling capability).

In T235796#5628056, @Jdlrobson wrote:

In T235796#5627369, @Esanders wrote:

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.

To clarify, this small style module only adds the "3D" label to the thumbnail. Clicking it still does not provide any 3D capabilities as that requires MMV to (also) load on mobile.

Is there a task for actually enabling 3D on mobile and/or for unifying MMV and the MobileFrontend re-implementation? I think MMV could become 10-100X smaller. It doesn't need to be a JS-heavy codebase, it was massively over-engineered at the time, and the MobileFrontend re-implementation demonstrates how far we can come with ~5KB instead of ~100KB of code (minified, decompressed).

This is tracked in T65504 (see also subtasks). I've expanded the description - please feel free to do so too! I've also tagged web team for now so at least one team is aware of it and tracking it (I think this fell through the cracks since nobody is maintaining Extension:MultimedIaViewer now)

The ext.3d.styles module should not be added unconditionally to every page and should set target mobileClosed, ResolvedPublicActions