Page MenuHomePhabricator

TMH audio player missing for clips inside <indicator>
Closed, ResolvedPublic

Description

The "Spoken Wikipedia" template on Dutch Wikipedia (nl.wikipedia.org) features an audio clip with small player through which the user can read to a spoken version of the article. This is similar to the "Featured article" and "Coordinates" indicators found on other wikis.

The template is at https://nl.wikipedia.org/wiki/Sjabloon:Gesproken_Wikipedia_klein and still uses the legacy JavaScript/CSS hack which requires Common.js hacks to eject the element from the page content and move it around (after page load) to the page header.

This is what <indicator> is for, but it seems that when trying to convert it to that syntax, the TMH player is no longer applied.

When editing the template, and switching it to <indicator>, and then previewing it on an article results in the following:

capture.png (1×2 px, 358 KB)

Event Timeline

Maybe the TMH JavaScript is scoped to #bodyContent or #mw-content-text?

Can you create a copy of the template that uses the form that fails? I'm uncertain what to modify from the description.

Looking at the TimedMediaHandler JS, it uses the wikipage.content hook which passes a jQuery object holding the content area in which to process elements. Anything outside the content area is not processed.

You can force a new player to process individually without regard for being in a wiki content area but it's a little funky and the API's not stable.

Maybe something like:

if ( mw.processEmbedPlayers ) {
  // For the old code, still deployed for most people
  mw.processEmbedPlayers( $( '#my-audio-element' ) );
} else {
  // For the new code, available through beta feature
  $( '#my-audio-element' ).loadVideoPlayer();
}

and that assumes the relevant modules are loaded, which are also unstable and changing. Hmm, a consistent API would be good here. :D

Or... perhaps MW should run wikipage.content against the contents of <indicator> as well?

Aye, that's the underlying issue indeed.

This affects all hook-triggered enhancements, in all places that aren't the main body content.

Such as sortable and collapsible tables inside indicators), albeit unlikely (but never say never...).

Or sortable content inside a DISPLAYTITLE.

TMH is probably the only enhanced feature one realistically use in an indicator, though, hence the one we found first.

I'd rather not make the the wikipage.content hook feature aware of the indicators feature explicitly. But, we might be able to work with a common class name. Such as mw-body-content or (ideally) mw-parser-output. Except the latter isn't set for indicators currently.

I wonder if this means TemplateStyles don't work in indicators either?

Since AFAIK extensions are meant to use wikipage.content for setup, I think any area where wikitext features are expected to work _should_ trigger the hook... A common class name would still require knowing when to run setup, and potentially one might want to update the indicator (what happens during edit preview?). So we either have to add a second hook that does the same thing as wikipage.content but is explicitly scoped to non-primary areas, or ...? Hmm, bears further thought.

I wonder if this means TemplateStyles don't work in indicators either?

Well, you would have to add the mw-parser-output class yourself for it to work. See T37247, https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/352835/ and T188443.

Krinkle triaged this task as Low priority.
Krinkle edited projects, added MediaWiki-User-Interface; removed MediaWiki-Parser.

There is a way around this now. Add the following to Common.js:

	mw.hook( 'wikipage.indicators' ).add( function ( $content ) {
		$content.find( 'video, audio' ).loadVideoPlayer();
	} );

There is a way around this now. Add the following to Common.js:

	mw.hook( 'wikipage.indicators' ).add( function ( $content ) {
		$content.find( 'video, audio' ).loadVideoPlayer();
	} );

What about adding that to TMH instead of Common.js? Kartographer also subscribes to both wikipage.content and wikipage.indicators.

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

[mediawiki/extensions/TimedMediaHandler@master] Allow page indicators to load videoJS player

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

TheDJ moved this task from Backlog/maint to In progress on the TimedMediaHandler board.
TheDJ edited projects, added VideoJS player; removed TimedMediaHandler.

This abuse of page indicators is really not great (they're absolutely never meant to be used for page content), but eh…

Change 785917 merged by jenkins-bot:

[mediawiki/extensions/TimedMediaHandler@master] Allow page indicators to load videoJS player

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