Page MenuHomePhabricator

Reduce Javascript Code Size per 2014 Mobile Team recommendation
Closed, DuplicatePublic

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
mediawiki/extensions/MultimediaViewermaster+3 -738
mediawiki/extensions/MultimediaViewermaster+7 -26
mediawiki/extensions/MultimediaViewermaster+0 -12
mediawiki/extensions/MultimediaViewermaster+23 -39
mediawiki/extensions/MultimediaViewermaster+20 -59
mediawiki/extensions/MultimediaViewermaster+2 -13
mediawiki/extensions/MultimediaViewermaster+6 -31
mediawiki/extensions/MultimediaViewermaster+99 -178
mediawiki/extensions/MultimediaViewermaster+65 -122
mediawiki/extensions/MultimediaViewermaster+58 -87
mediawiki/extensions/MultimediaViewermaster+29 -46
mediawiki/extensions/MultimediaViewermaster+7 -123
mediawiki/extensions/MultimediaViewermaster+46 -53
mediawiki/extensions/MultimediaViewermaster+53 -60
mediawiki/extensions/MultimediaViewermaster+98 -115
mediawiki/extensions/MultimediaViewermaster+4 -2
mediawiki/extensions/MultimediaViewermaster+0 -6
mediawiki/extensions/MultimediaViewermaster+18 -40
mediawiki/extensions/MultimediaViewermaster+1 -122
mediawiki/extensions/MultimediaViewermaster+0 -14
mediawiki/extensions/MultimediaViewermaster+0 -50
mediawiki/extensions/MultimediaViewermaster+155 -784
mediawiki/extensions/MultimediaViewermaster+22 -32
mediawiki/extensions/MultimediaViewermaster+78 -83
mediawiki/extensions/MultimediaViewermaster+21 -21
mediawiki/extensions/MultimediaViewermaster+45 -107
mediawiki/extensions/MultimediaViewermaster+28 -123
mediawiki/extensions/MultimediaViewermaster+8 K -8 K
mediawiki/extensions/MultimediaViewermaster+1 -219
mediawiki/extensions/MultimediaViewermaster+80 -71
mediawiki/extensions/MultimediaViewermaster+55 -0
Show related patches Customize query in gerrit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

The mobile version is pretty 'dumb'. The desktop version does a whole slew of things that mobile doesn't.

It has its own queue system, animations, progress bar logic, it analyzes thumbnail sizes to find suitable ones, has a BUNCH of license and template handling code for the metadata panels which seems very expensive and the list goes on.

But if we really want to strip bytes, I think we have to check what code we can replace with new native browser functionality. Pure simplification and getting rid of some of the many wrappers around functionality that we have in there right now.
One area i see is the animations for instance, some of them currently use JS instead of CSS.

Change #1032556 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Extract mmv.ui.restriction module

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

Change #1031499 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Monitor bundlesizes in MultimediaViewer

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

Change #1032556 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Extract mmv.ui.restriction module

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

It seems MMV now clocks in at 40kb (mmv) compared to 3.1KB on mobile (mobile.mediaViewer module) for code loaded when an image is clicked.

Those number refer to the gzip size? When loading https://en.m.wikipedia.org/w/load.php?lang=en&modules=mmv, or https://en.m.wikipedia.org/w/load.php?lang=en&modules=mobile.mediaViewer, respectively, I obtain 41kB, or 4.2kB gzip-compressed and 194kB, or 8.8kB uncompressed (no transfer compression applied, but JS minified).

They should refer to gzip. https://en.m.wikipedia.org/w/load.php?lang=en&modules=mmv is currently 38.83 KB when compressed. How are you measuring? I added a bundle size test which checks the gzip size so we can now monitor it. After your change to extract mmv.ui.restriction we are down to 30kb (💪)

With Gzipping I don't think the requires will make much of a dent (maybe 1kb or so?), but we should do it anyway for readability:

  1. it would be good to simplify the file names
  2. and drop the ( function () { .. }() ); closures) (patch-welcome).
  3. I think it would also be more readable to get rid of the files like mmv/provider/mmv.provider.js which don't serve much of a purpose and just add indirection to our code - we should require directly e.g.
const Api = require( './provider/mmv.provider.Api.js' );

instead of

const { Api } = require( './provider/mmv.provider.js' );

For reducing size, as @TheDJ says the desktop version is just more feature rich. I think 15kb would be a good number to chase towards.

I am seeing some most likely dead code that we should eliminate: correctEW, isRTL, setInlineStyle, setTimer, clearTimer and resetTimer in mmv.ui.js all appear to only get used in test code! It's probably worth running some analysis on the code to see if there is any more code like that.

We can probably also merge some of the mmv modules.

Like, head, bootstrap, starting… i think that can all be one RL module right ? The dialog modules can probably be merged into a single module.

Change #1034188 had a related patch set uploaded (by TheDJ; author: TheDJ):

[mediawiki/extensions/MultimediaViewer@master] Remove unused code from mmv.ui

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

Change #1034188 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Remove unused code from mmv.ui

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

Change #1034573 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Remove index.js export indirection

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

Change #1034577 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Remove IIFE

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

Change #1034577 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Remove IIFE

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

Change #1034573 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Remove index.js export indirection

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

Change #1035028 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Merge download/share/embed into mmv.ui.reuse module

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

Change #1035033 had a related patch set uploaded (by TheDJ; author: TheDJ):

[mediawiki/extensions/MultimediaViewer@master] Use CSS transitions for the progressbar.

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

Change #1035560 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Move dialog/reuse dialog code to mmv.ui.reuse module

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

Change #1035028 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Merge download/share/embed into mmv.ui.reuse module

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

I used Chrome developer tools today to identify unused code. Here are some notes from my findings:

  • Move getShortLink mmw.EmbedFileFormatter.js
  • isFree out of License and to utility functions inside mmv.ui.dialog

Unused functions (dead code not used anywhere):

  • Image::getThumbUrl
  • Repo::getArticlePath
  • GuessedThumbnailInfo::displayableExtensions
  • GuessedThumbnailInfo::vectorExtensions
  • Abstract methodds on class UiElement
  • MultimediaViewer::preloadFullscreenThumbnail - function is commented out. Delete this and associated code

Canvas::getLightboxImageWidthsForFullscreen

  • TruncatableTextField::repaint

Move to mmv.ui.reuse:

  • Repo::getSiteLink
  • License::needsAttribution
  • OptionsDialog and associated code does not need to be loaded on page load.

Notes:

  • A lot of the unused code relates to events that only occur when you do things. Perhaps all events could be delayed to the dialog right /after/ the initial display?
  • Most of GuessedThumbnailInfo is not utilized on most page views. Would be good to move out to a module of its own that is loaded when needed.
  • Most of Dialog code openDialog, startListeningToOutsideClick, closeDialog, stopListeningToOutsideClick, unattach etc.. could be loaded on demand. There is so much code unused on page load relating to setting up these events. We could replace them with a simple piece of code that adds a single event and data attribute pointing to the dialog needed that loads dialog code down when needed.
    • Replaced with a simple openDialog proxy method that loads the code and then executes it. The Dialog code is not used for the display of the image itself :-) Maybe Dialog themselves could be pulled out into a separate module? The ReuseDialogProxy could be a good pattern to generalize and use for all dialogs now we have it.
  • methods blur and unblurWithAnimation in Canvas
  • isCommons is only used in mmv.ui.stripeButtons.
  • MetadataPanel::setRestrictions and could inside MetadataPanel::createRestriction could be moved to mmv.ui.restriction

I used Chrome developer tools today to identify unused code. Here are some notes from my findings:
Unused functions (dead code not used anywhere):

  • Image::getThumbUrl
  • Repo::getArticlePath
  • GuessedThumbnailInfo::displayableExtensions
  • GuessedThumbnailInfo::vectorExtensions
  • Abstract methodds on class UiElement
  • MultimediaViewer::preloadFullscreenThumbnail - function is commented out. Delete this and associated code

Canvas::getLightboxImageWidthsForFullscreen

  • TruncatableTextField::repaint

Some of this is definitely not true. They are simply in code branches not hit by the browser because they are only used in quite specific situations of either browser, setup of mediawiki, or the thumbnail in question.

  • OptionsDialog and associated code does not need to be loaded on page load.

I'm pretty sure it is there for if you click the cogwheel on the file page.

Notes:

  • A lot of the unused code relates to events that only occur when you do things. Perhaps all events could be delayed to the dialog right /after/ the initial display?
  • Most of GuessedThumbnailInfo is not utilized on most page views. Would be good to move out to a module of its own that is loaded when needed.

This depends on your configuration. On WMF wikis, it actually most of the times DOES use guessing (to avoid doing an api call). But the default is for it to be off, because you need to have 404 thumbnail generations setup, which many installs (incl dev setups) do not.

  • Most of Dialog code openDialog, startListeningToOutsideClick, closeDialog, stopListeningToOutsideClick, unattach etc.. could be loaded on demand. There is so much code unused on page load relating to setting up these events. We could replace them with a simple piece of code that adds a single event and data attribute pointing to the dialog needed that loads dialog code down when needed.
    • Replaced with a simple openDialog proxy method that loads the code and then executes it. The Dialog code is not used for the display of the image itself :-) Maybe Dialog themselves could be pulled out into a separate module? The ReuseDialogProxy could be a good pattern to generalize and use for all dialogs now we have it.

This is because of the OptionsDialog being loaded i think. Maybe we should look into a simpler way to turn of mediaviewer ?

Additionally, before we do too much.. It would be worth considering simply rewriting some of this stuff in vue. A much bigger effort but might be better than writing things twice.

  • methods blur and unblurWithAnimation in Canvas

Gone now luckily

  • isCommons is only used in mmv.ui.stripeButtons.

I do think those are very much repo characteristics though. But we might be able to simplify it a bit ?

@Jdlrobson How are you measuring?

I was using Firefox's network debugger and obtained the value from the "Transferred" column. But this seems to include the HTTP headers, too. You're reading the Content-Length HTTP header, right?

> curl --head --compressed --silent 'https://en.m.wikipedia.org/w/load.php?lang=en&modules=mmv' | grep content-length
content-length: 39765

@TheDJ It would be worth considering simply rewriting some of this stuff in vue. A much bigger effort but might be better than writing things twice.

Vue pulls in 50 kB of additional code. AFAIK, Vue seems to be loaded on en.wikipedia.org and commons.wikimedia.org already. However, the mobile version en.m.wikipedia.org does not use Vue at the moment (?). Since we try to reduce the code size especially for mobile, switching to Vue would be disadvantageous regarding code size?

> curl --head --compressed --silent 'https://en.m.wikipedia.org/w/load.php?lang=en&modules=vue' | grep content-length
content-length: 50468

Change #1036581 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Api: inline getQueryField function

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

Change #1036606 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Use optional chaining

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

Change #1036724 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Use mw.msg shortcut

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

Change #1037027 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Remove FileRepoInfo API and Repo model

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

Change #1035560 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Move dialog/reuse dialog code to mmv.ui.reuse module

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

Change #1036606 abandoned by Simon04:

[mediawiki/extensions/MultimediaViewer@master] Use optional chaining

Reason:

Not supported by all relevant browsers.

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

Change #1035033 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Use CSS transitions for the progressbar.

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

Change #1037027 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Remove FileRepoInfo API and Repo model

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

Change #1036581 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Api: remove obsolete getQueryField function

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

Change #1043196 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Remove obsolete class IwTitle

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

Change #1043219 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Remove obsolete property LightboxImage.filePageLink

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

Change #1043221 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Remove obsolete fields imageDisplayedCount and metadataDisplayedCount

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

Change #1043221 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Remove obsolete fields imageDisplayedCount and metadataDisplayedCount

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

Change #1043196 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Remove obsolete class IwTitle

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

Change #1043219 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Remove obsolete property LightboxImage.filePageLink

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

Change #1047568 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Inline MultimediaViewer.fetchSizeIndependentLightboxInfo

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

Change #1047569 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Inline variables in ImageModel.newFromImageInfo

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

Change #1047570 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Inline MultimediaViewer.fetchThumbnailForLightboxImage

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

Change #1047571 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Remove unused MultimediaViewer.preloadFullscreenThumbnail

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

Change #1047572 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Inline MultimediaViewerBootstrap.getViewer

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

Change #1047573 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Remove obsolete MultimediaViewerBootstrap.statusInfoDialog

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

Change #1047573 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Remove obsolete MultimediaViewerBootstrap.statusInfoDialog

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

Change #1061076 had a related patch set uploaded (by TheDJ; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Fix commons icon on commons

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

Reminder: todo rewrite the modules to make use of RL's config.json logic / virtual files provided by PHP, instead of using our own config object.

Change #1061076 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Fix commons icon on commons

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

Change #1061142 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Use mw.storage directly

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

Change #1061310 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Use mw.user directly

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

Change #1061314 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Use mw.config directly

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

Change #1061325 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Use mw.Api.prototype.saveOption directly

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

Change #1061326 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Use mw.storage.get/set/remove directly

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

Change #1061341 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Replace wgMultimediaViewer with config.json packageFiles

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

Change #1061365 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Make remaining functions in Config class static

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

Change #1061142 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Use mw.storage directly

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

Change #1061310 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Use mw.user directly

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

Change #1061314 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Use mw.config directly

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

Change #1061325 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Use mw.Api.prototype.saveOption directly

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

Change #1061326 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Use mw.storage.get/set/remove directly

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

Change #1061341 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Replace wgMultimediaViewer with config.json packageFiles

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

Change #1061365 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Make remaining functions in Config class static

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

I would be making it possible to enable the desktop MMV on mobile and running some synthetic tests.

To do this we'll need the following

  1. A feature flag that allows us to turn off MobileFrontend's native media viewer and enable MultimediaViewer's
  2. The desktop MMV should allow cycling images e.g. it should be possible to get from the first image to the last image. This is feature on mobile and we should aim to get parity.

Any concerns with this?

Yay!

The desktop MMV should allow cycling images e.g. it should be possible to get from the first image to the last image.

I don't have any concerns. See T77877 and its abandoned patch https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MultimediaViewer/+/339942 for some reservations dating back to 2014 and 2017.

Change #1078732 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Remove MMV options dialog in favour of Special:Preferences

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

Change #1078732 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Remove MMV options dialog in favour of Special:Preferences

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

Change #1080427 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Merge MultimediaViewer bootstrap modules

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

Change #1080428 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] Move HtmlUtils from mmv.bootstrap to mmv module

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

Change #1080427 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Merge MultimediaViewer bootstrap modules

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

Change #1080853 had a related patch set uploaded (by Simon04; author: Simon04):

[mediawiki/extensions/MultimediaViewer@master] MMV options dialog: remove leftover code

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

Change #1080853 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] MMV options dialog: remove leftover code

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

Change #1047571 abandoned by Simon04:

[mediawiki/extensions/MultimediaViewer@master] Remove unused MultimediaViewer.preloadFullscreenThumbnail

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

Change #1047569 abandoned by Simon04:

[mediawiki/extensions/MultimediaViewer@master] Inline variables in ImageModel.newFromImageInfo

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

Change #1047568 abandoned by Simon04:

[mediawiki/extensions/MultimediaViewer@master] Inline MultimediaViewer.fetchSizeIndependentLightboxInfo

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

Change #1047572 abandoned by Simon04:

[mediawiki/extensions/MultimediaViewer@master] Inline MultimediaViewerBootstrap.getViewer

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

Change #1047570 abandoned by Simon04:

[mediawiki/extensions/MultimediaViewer@master] Inline MultimediaViewer.fetchThumbnailForLightboxImage

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