Page MenuHomePhabricator

VideoJS needs to be lazy-loaded on click before we can release it by default
Open, HighPublic0 Story Points

Description

mw.inspect size report

Desktop

namesizesizeInBytes
0"ext.tmh.video-js""818.2 KiB"8378601
1"oojs-ui-core""165.9 KiB"1698322
2"jquery""147.0 KiB"150554

Mobile

namesizesizeInBytes
0"ext.tmh.video-js""818.2 KiB"8378601
1"jquery""147.0 KiB"150554

Oh dear.

Details

Related Gerrit Patches:
mediawiki/extensions/TimedMediaHandler : masterTransform audio players in video.js mode again
mediawiki/extensions/TimedMediaHandler : masterReducing size of video.js payload
mediawiki/extensions/TimedMediaHandler : masterLazy-load JS payload & use OOUI dialog in video.js playback mode

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 17 2019, 8:48 PM
TheDJ added a subscriber: TheDJ.Jul 23 2019, 7:17 PM

so initially we chose this route, to make sure we had full HTML5 fallback.. The technology of lazy loading is somewhat incompatible with html5 fallback content as the controls are always presented.. you can add lots of logic on top to hide the native player and everything, but what if someone already started native html5 playback before your 'lazy load' logic start overlaying it ?

It also causes problems for parsoid serialization, having to know multiple forms of dom representation for the same content and additional overhead.
We should not forget that the old module was once lazy loaded, because RL modules were present on ALL pages, whether they had video or not and had to look at the DOM of a page to figure that out..

I think we should however also look at the size of video.js RL module. We are currently including the full dist version, incl webvtt parser and presentation fallback for platforms that do not support webvtt, Dash and HLS handling, Cea608 handling, mux.js, several components we don't use etc etc..

by handcrafting our own bundle using webpack and packagefiles, we could probably significantly reduce the file initial payload size. I set off doing this at the hackathon, but then got horribly stuck inside trying to lazy load ogvjs into that and got so frustrated that I think i lost the patch/branch i was working on.

TheDJ added a comment.Jul 23 2019, 7:26 PM

Found that experiment back: https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/TimedMediaHandler/+/525145/

Not fun. We have 4 ecosystems that need to communicate (videojs, videojs plugins, mediawiki and ogvjs ), using respectively: cjs+rollup, <whatever the plugin author wanted to use>, package modules+webpack, and umd+webpack

and then we need lazyloading working on top of that...

brion added a subscriber: brion.Jul 29 2019, 9:46 PM

We can probably reduce the size for now by using the video.js core distribution (without the http-streaming component which we don't use yet) and using more aggressive minification (rather than using our own minifier, which doesn't shorted variable names and such).

As for lazy loading, worst case we can use some kind of click handler I think to display our pretty icon and catch events and then load up the rest of bits, even with a native <video> element showing its poster image. Will experiment with this later if you don't get to it first. :)

Change 529819 had a related patch set uploaded (by Brion VIBBER; owner: Brion VIBBER):
[mediawiki/extensions/TimedMediaHandler@master] WIP Reducing size of video.js payload

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

brion added a comment.Aug 12 2019, 6:32 PM

Patch switches to the core distribution (no http-streaming component) and the more aggressively pre-minified version. Not quite complete, as this means we can't patch in the text track lazy-loading on top as easily.

Should either get that pushed upstream or make a work fork we build from, or both.

Quick fly-by note from talking about this with @Ladsgroup at the hackathon....

... it looks to me like the old Kaltura-based player is not lazy-loaded either. As such, if we can assert that the full videojs payload is smaller than the full Kaltura + jqueryUI payload, then this does not need to block enabling of TMH-videojs in production by default.

I noticed this when inspecting the performance of articles like https://en.wikipedia.org/wiki/Sweden which load all of jqueryUI, apparently due to the anthem audio-clip queuing the TMH-kaltura player unconditionally (not lazy-loaded).

TheDJ added a comment.EditedAug 14 2019, 8:31 AM

old Kaltura player is not lazy loaded if you use audio-only or videos over $wgMinimumVideoPlayerSize

brion added a comment.Aug 14 2019, 2:22 PM

For a video that triggers player setup but hasn't actually been played, in Chrome/Firefox where no ogv.js shim is needed, they seem to weigh in (as reported from mw.inspect, so uncompressed) at around 293 KiB for MwEmbed+jquery.ui and 288 KiB for video.js with the core.min.js variant.

Note MwEmbed also has a 13 KiB sprite PNG for icons (none of them hi-DPI), while video.js uses a scalable icon font embedded in the CSS for the widgets.

We could possibly trim it down further by lazy-loading the subtitle rendering component, but that's less of a big deal.

We still have the issue that use of very small video embeds currently doesn't load the player until popup while this will pre-load the player components, so total usage may go up (but any use of an audio player already engages it, so eh)

Yeah, I think on the whole, I'd be okay with video.js going ahead as-is (from a perf perspective). With lazy-loading being something we'll get to later (as soon as possible, but not blocking).

brion added a comment.Aug 14 2019, 2:51 PM

Great, I'll tidy up this patch in a bit (once I get the subtitle lazy-loading patched back in) and we'll worry about lazy-loading later.

Krinkle added a comment.EditedOct 1 2019, 7:27 PM

Does Video.js release imply enablement on MobileFrontend? If so, we may want to require lazy-loading on click after all. Or keep it desktop-only at first.

When I last looked into it a couple months ago, I was surprised to see jquery-ui still loaded on e.g. the perf-monitored "Sweden" article. Which was due to TMH loading it unconditionally on page load for the player in the infobox. The payload would be quite a sizeable regression on a lot of popular pages. on mobile. In particular because it also affects audio tracks which lots of articles have e.g. for their pronunciation or national anthem.

At the time, the size of the TMH payload almost entirely represents the difference in JS payload size between desktop and mobile for such articles.

/cc @Jdlrobson

Krinkle added a subscriber: Jdlrobson.
ovasileva claimed this task.Oct 2 2019, 8:45 AM
ovasileva moved this task from Incoming to Needs Prioritization on the Readers-Web-Backlog board.
brion added a comment.Oct 2 2019, 4:59 PM

Does Video.js release imply enablement on MobileFrontend? If so, we may want to require lazy-loading on click after all. Or keep it desktop-only at first.

Yes, it would be enabled for both desktop and mobile targets at present; there's no differential behavior between the two targets for video.js mode.

When I last looked into it a couple months ago, I was surprised to see jquery-ui still loaded on e.g. the perf-monitored "Sweden" article. Which was due to TMH loading it unconditionally on page load for the player in the infobox. The payload would be quite a sizeable regression on a lot of popular pages. on mobile. In particular because it also affects audio tracks which lots of articles have e.g. for their pronunciation or national anthem.

Audio is particularly tricky because it shows a player by default and does not provide any opportunity for lazy-loading at present. If we want lazy-loading for audio players, we have to come up with a UI for it -- either a temporary placeholder UI that's replaced by the video.js, or ditch video.js in favor of custom control code that is smaller and can lazy-load things like subtitle management and the ogv.js shim only when actively needed.

That's not impossible, but doing a custom control UI is new work and requires design and implementation, and as a reminder there is no team dedicated to audio/video work. I would need to borrow some designer time or else make up designs myself. ;)

Do we want to reuse wgMinimumVideoPlayerSize with this new lazy load setup ?
https://gerrit.wikimedia.org/r/550926

Change 550926 had a related patch set uploaded (by Jforrester; owner: Brion VIBBER):
[mediawiki/extensions/TimedMediaHandler@master] Lazy-load JS payload & use OOUI dialog in video.js playback mode

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

Change 550926 merged by jenkins-bot:
[mediawiki/extensions/TimedMediaHandler@master] Lazy-load JS payload & use OOUI dialog in video.js playback mode

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

@TheDJ let's try using it without wgMinimumVideoPlayerSize and drop it back in if we decide the always-transformation is confusing.

Ok, we've got the dialog merged which lets us lazy-load. Awesome! Tomorrow I'll fix up the other patch to use the latest video.js release -- the pre-minified sources compress much better than our minified version, and we no longer need to patch the JS source so it's easy to use it. I'll need to make one small change to activate the option for lazy-loading subtitle tracks.

brion added a comment.Wed, Nov 27, 8:33 PM

Updated https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/TimedMediaHandler/+/529819/ to use the better minified package. Much smaller JS payload now. :)

I think the audio player will be a problem. Native players are pretty big and we often have very restrictive spacing. Don’t know a good design either though..

I think the audio player will be a problem. Native players are pretty big and we often have very restrictive spacing. Don’t know a good design either though..

Agreed, in some browsers they're *huge* and it's really inconsistent for styling. I also really want to get captions back for them...

What I'm currently thinking is some kind of nice compact "play" icon that pops up a smaller version of the video dialog. This could be sized to include space for captions if captions are available, and we could run them in the dialog instead of in a separate div.

However I'm not sure if I should have a square-ish stubby play button, or something that is wider similar to the actual transformed player to avoid disrupting style choices made around the old player... Any thoughts/preference?

Change 553442 had a related patch set uploaded (by Brion VIBBER; owner: Brion VIBBER):
[mediawiki/extensions/TimedMediaHandler@master] Transform audio players in video.js mode again

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

Change 529819 merged by jenkins-bot:
[mediawiki/extensions/TimedMediaHandler@master] Reducing size of video.js payload

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