Page MenuHomePhabricator

Parsoid's timed media output is missing data- attributes
Open, MediumPublic

Description

See the latest sync in https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/+/776006

I'm seeing,

data-durationhint="5" data-mwtitle="Video.ogv" data-mwprovider="local"

and

data-bandwidth="590013" data-framerate="30"

Event Timeline

Arlolra triaged this task as Medium priority.Mar 31 2022, 11:01 PM
Arlolra moved this task from Needs Triage to Media Structure on the Parsoid board.
Arlolra renamed this task from Parsoid's timed media output is missing the data-durationhint attribute to Parsoid's timed media output is missing the data- attributes.Mar 31 2022, 11:04 PM
Arlolra renamed this task from Parsoid's timed media output is missing the data- attributes to Parsoid's timed media output is missing data- attributes.
Arlolra updated the task description. (Show Details)

It looks like some of these are being removed in T199129

Change 933177 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/services/parsoid@master] Remove attributes deprecated in TMH

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

data-durationhint="5" data-mwtitle="Video.ogv" data-mwprovider="local"

These likely will remain.

  • mwtitle and mwprovider: needed to make it easier to find the accompanying description page. I do believe that parsoid has some of this info as well on images and I know the MultimediaViewer infers this information from the image url, so perhaps this should be standardised ?
  • Duration hint: this is required to show duration before the player is initialised
  • Optional data-transcodekey: this is an identifier for the type of derivative source. It is present on all derivatives (transcodes) of an a/v item (source), but not on the original source. Generally the original source is present in the html, but this is not required and sometimes there is only an original and no derivative.

Removing:

  • data-bandwidth and data-framerate: These used to be required for source selection and source ordering in the player, but these are now mostly done serverside and the client no longer has to know about this.
  • data-title and data-shorttitle: These were used as labels in the menu's of the old JS player. The new JS player now only relies on the data-transcodekey to find the associated interface messages.

Change 959030 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/services/parsoid@master] Add durationhint to timed media

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

Change 959030 merged by jenkins-bot:

[mediawiki/services/parsoid@master] Add durationhint to timed media

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

Change 960644 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/vendor@master] Bump parsoid to 0.18.0-a25

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

Change 960644 merged by jenkins-bot:

[mediawiki/vendor@master] Bump parsoid to 0.18.0-a25

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

Note the File:Undefined from https://it.wikipedia.org/wiki/Timor_Est?useparsoid=1 while verifying T346703#9204822

<span class="mw-tmh-player audio mw-file-element"><audio preload="none" height="32" width="220" resource="./File:East_Timorese_national_anthem.wav" data-durationhint="130" class="" id="undefined_placeholder" disabled="disabled" tabindex="-1"></audio><a class="mw-tmh-play" href="/wiki/File:Undefined" title="Riproduci file multimediale" role="button"><span class="mw-tmh-play-icon"></span></a><span class="mw-tmh-duration mw-tmh-label" aria-label="2 minuti e 10 secondi">2:10</span></span>
  • Optional data-transcodekey: this is an identifier for the type of derivative source. It is present on all derivatives (transcodes) of an a/v item (source), but not on the original source. Generally the original source is present in the html, but this is not required and sometimes there is only an original and no derivative.

In T133670, Parsoid implemented,

			$fromFile = isset( $o['transcodekey'] ) ? '' : '-file';
			if ( $hasDimension ) {
				$source->setAttribute( 'data' . $fromFile . '-width', (string)$o['width'] );
				$source->setAttribute( 'data' . $fromFile . '-height', (string)$o['height'] );
			}

but TMH does,

		$prefixedSourceAttr = [
			'width',
			'height',
			'transcodekey',
		];
		foreach ( $mediaSources as &$source ) {
			foreach ( $source as $attr => $val ) {
				if ( in_array( $attr, $prefixedSourceAttr, true ) ) {
					$source[ 'data-' . $attr ] = $val;
					unset( $source[ $attr ] );
				}
			}
		}

It looks like T133670 was closed before ever being implemented in TMH. Which direction should we go in consolidating? data-transcodekey is being used in resources/ext.tmh.player.inline.js, so that I will add regardless.

Change 961904 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/services/parsoid@master] Add optional data-transcodekey to media sources

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

  • mwtitle and mwprovider: needed to make it easier to find the accompanying description page. I do believe that parsoid has some of this info as well on images and I know the MultimediaViewer infers this information from the image url, so perhaps this should be standardised ?

Parsoid sets this as the resource attribute on the media element,
https://github.com/wikimedia/mediawiki-extensions-TimedMediaHandler/blob/master/includes/TimedMediaTransformOutput.php#L198-L203

Since that's at least present for timed media thumbs, the uses of mwtitle can probably be rewritten with that. I will look into doing that.

MultimediaViewer works by looking for the mw-file-description class. When that's present, the link around an image is pointing to the file description page so it uses the href of that link.

Change 933177 merged by jenkins-bot:

[mediawiki/services/parsoid@master] Remove attributes deprecated in TMH

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

Change 962706 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.18.0-a26

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

Change 962706 merged by jenkins-bot:

[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.18.0-a26

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

Change 961904 merged by jenkins-bot:

[mediawiki/services/parsoid@master] Add optional data-transcodekey to media sources

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

Change 964566 had a related patch set uploaded (by Jgiannelos; author: Jgiannelos):

[mediawiki/vendor@master] Bump wikimedia/parsoid to v0.18.0-a28

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

Change 964566 merged by jenkins-bot:

[mediawiki/vendor@master] Bump wikimedia/parsoid to v0.18.0-a28

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

So let's do another check on where we are...

  1. Old parser sets data-mwtitle and data-mwprovider on <video> and <audio>. With parsoid the first becomes ./resource
  2. Parsoid sets data-file-width and data-file-height for the original, but older parser set data-height / data-width
  3. <tracks> set data-mwtitle=""
  4. The order of the sources differs between the two parsers
  1. I'll write a fix for this
  2. I'm a little on the fence here, as it might be that there is no <source> for the original at some point...
  3. I think these mwtitle can be removed from parsoid (not sure how they get there)
  4. Not sure how much this still matters...

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

[mediawiki/extensions/TimedMediaHandler@master] Add resource attribute to video elements

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

Change 977278 abandoned by TheDJ:

[mediawiki/extensions/TimedMediaHandler@master] Add resource attribute to video elements

Reason:

This patch does not seem like the right way forward.

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

@Arlolra how do i configure my wiki to use native parsoid rendering ? i can't find that anywhere in the documentation (Like in the beta on wmf wikis).

Have a look at the Migration to Parsoid read views section of the README of the ParserMigration extension and let me know if that works for you
https://github.com/wikimedia/mediawiki-extensions-ParserMigration

There's also $wgParserMigrationEnableQueryString, which is useful
https://github.com/wikimedia/mediawiki-extensions-ParserMigration/blob/master/extension.json#L17-L20

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

[mediawiki/extensions/TimedMediaHandler@master] Use resource instead of mwtitle

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

Change #1017149 merged by jenkins-bot:

[mediawiki/extensions/TimedMediaHandler@master] Use resource instead of mwtitle

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

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

[mediawiki/extensions/TimedMediaHandler@REL1_42] Use resource instead of mwtitle

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

Change #1025316 merged by jenkins-bot:

[mediawiki/extensions/TimedMediaHandler@REL1_42] Use resource instead of mwtitle

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

What remains to be done on this task? Is this a still a blocker for rolling out parsoid read views on some wikis?

I'd recommend closing out this bug and opening a new task for the remaining work (if any).

I don't think there is a blocker for that. The only remaining issue is that the player wont directly go to commons if it is a commons file when using Parsoid, because it doesn't know about shared file repos.
It’s not ideal but acceptable.

Additionally, there is a bit of an issue with the thumb= and links= logic, but those are super rare icw with videos as far as om aware, and behavior for mixing those options has never been really well defined to begin with.