Page MenuHomePhabricator

Adjust TimedMedia url handling (getAPIData) to match legacy parser
Open, MediumPublic

Description

Parsoid calls $mto->getAPIData() to get the audio/video data, which is defined here:
https://github.com/wikimedia/mediawiki-extensions-TimedMediaHandler/blob/master/includes/TimedMediaTransformOutput.php#L607

It looks like this was an interface we added just for Parsoid, in T51896: In Parsoid and MW API, implement handling for file types that do not have thumbnails, such as audio ogg files.

But the HTML output used by the legacy parser is based on getMediaSources(), which is private (sigh): https://github.com/wikimedia/mediawiki-extensions-TimedMediaHandler/blob/master/includes/TimedMediaTransformOutput.php#L562

The important difference is that getMediaSources() has:

$source['src'] .= $this->getTemporalUrlHash();

That's a #t=.... parameter, which we're presumably not matching in Parsoid at this time. Maybe not the exact same issue as T235231: Parsoid/JS video tag has a "seek" parameter in the URL that Parsoid/PHP video tag output doesn't but the same idea.

getAPIData() also has a bunch of wfExpandUrl() calls we'd like to avoid (some hidden by a "['fullurl'] option to WebVideoTranscode::getSources()`). (See T235217: Parsoid should use protocol-relative URLs for media.)

Two possible solutions:

  1. Add a parameter to getAPIData(), something like $noExpandUrl=false or the ['fullurl'] option from WebVideoTranscode::getSources(), that avoids the wfExpandUrl calls and adds the getTemporalUrlHash()

    a) Perhaps getTemporalUrlHash() should actually always be added to getAPIData() regardless
  1. Make getMediaSources() non-private, and call that directly from our DataAccess. The other necessary method, getTextHandler() is already public.

As a short-term fix we could also just to strip the protocol unilaterally, but that won't match the wiki's configured preferences.

In any case, it appears that the TimedMediaOutput::toHtml method would be deprecated over time and eventually removed -- although maybe we'll need to keep it around for transition to T118517: [RFC] Use <figure> for media.

Details

Related Gerrit Patches:
mediawiki/extensions/ParsoidBatchAPI : masterAdd optional parameter to getAPIData() call
mediawiki/extensions/TimedMediaHandler : masterAdd url-format options to TimedMediaTransformOutput::getAPIData()

Event Timeline

cscott created this task.Nov 8 2019, 6:47 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 8 2019, 6:47 PM
cscott renamed this task from Adjust media handling to match legacy parser to Adjust TimedMedia url handling (getAPIData) to match legacy parser.Nov 8 2019, 6:52 PM

Change 550547 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/extensions/TimedMediaHandler@master] Add url-format options to TimedMediaTransformOutput::getAPIData()

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

Change 550548 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/extensions/ParsoidBatchAPI@master] Add optional parameter to getAPIData() call

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

Change 550547 merged by jenkins-bot:
[mediawiki/extensions/TimedMediaHandler@master] Add url-format options to TimedMediaTransformOutput::getAPIData()

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

ssastry triaged this task as Medium priority.Nov 14 2019, 11:09 PM
ssastry edited projects, added Parsoid; removed Patch-For-Review, Parsoid-PHP.

Change 550548 merged by jenkins-bot:
[mediawiki/extensions/ParsoidBatchAPI@master] Add optional parameter to getAPIData() call

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