Page MenuHomePhabricator

Adding a map to a page comes with performance penalty of loading entirety of OOUI
Closed, ResolvedPublicBUG REPORT

Description

Steps to replicate the issue (include links if applicable):

What happens?:
The module has been loaded on page load. It seems just for the purpose of a fullscreen button

What should have happened instead?:
It should make use of Codex CSS component to reduce the amount of code that needs to be loaded on page load
https://doc.wikimedia.org/codex/latest/components/demos/icon.html#icon-color

Software version (on Special:Version page; skip for WMF-hosted wikis like Wikipedia):

Other information (browser name/version, screenshots, etc.):

Event Timeline

For context: We worked on this extension in our past WMDE-GeoInfo-FocusArea. We are very interested but unfortunately can't make promisses. The issue appears to be quite old. I believe it goes back to 2016 when the static mode for high-traffic wikis was originally implemented, see T148070 and https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Kartographer/+/317018/8/modules/staticframe/staticframe.js#84. I guess the assumption back then was that OOUI will be loaded anyway on every page load. This is apparently not always true any more since we are slowly migrating to Codex and #desktop_improvements_vector_2022.

The ResourceLoader modules already attempt to do exactly this, and the only direct OOUI dependency in static mode is currently oojs-ui.styles.icons-media which was not supposed to pull in the entire bundle. +1 to switching to codex styles.

Okay, I found that it's actually the dependency on mediawiki.router which depends in turn on oojs. This module provides OO.Router and uses OO.Registry.

Router is useful even before the fullscreen map is opened, it seems that its main purpose is to make it possible to open an URL like https://en.wikipedia.org/wiki/User:Jdlrobson/sandbox/Machu#/map/0 and have the fragment automatically trigger a fullscreen map chosen from among the maps on the page.

I would like to turn this into a bug on mediawiki.router: either provide it as a standalone module, or decouple some of the OOUI-js dependencies until it only pulls in some minimum of the core OOUI modules.

Okay, I found that it's actually the dependency on mediawiki.router which depends in turn on oojs. This module provides OO.Router and uses OO.Registry.

oojs is not oo.ui though ? Where does that then come from ? Did someone merge the oojs and the ooui resource modules or something?

Another mitigation approach to take here is to load the library when the map is scrolled into view. The leaflet library is also pretty big and loaded on page load.

You can reference the new Charts extension which does this using an IntersectionObserver to load echarts only when needed

The leaflet library is also pretty big and loaded on page load.

Only on instances with dynamic maps right ? Otherwise something heavily regressed somewhere along the line

Wait a sec. Are we talking parsoid mode or old parser mode ? That changes how and when modules get added, perhaps Parsoid never implemented this correctly ?

$content.find('.mw-kartographer-map[data-mw-kartographer], .mw-kartographer-map[data-mw="interface"]')
.each(function(index) {
    const container = this;
    const $container = $(container);

    mw.loader.using('oojs-ui', () => {
        const button = new OO.ui.ButtonWidget({
            // In static mode this button is just a visual hint but doesn't have its own action
            tabIndex: -1,
            icon: 'fullScreen',
            framed: true
        });

Its the buttonwidget being loaded. I remember we looked at this before at some point.

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

[mediawiki/extensions/Kartographer@master] Switch Kartographer fullscreen button to Codex

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

Change #1103502 merged by jenkins-bot:

[mediawiki/extensions/Kartographer@master] Switch Kartographer fullscreen button to Codex

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

Volker_E claimed this task.