Page MenuHomePhabricator

[BUG] Audio file shows incomplete toolbar in an article page
Closed, ResolvedPublicBUG REPORT

Description

Steps to Reproduce:

  1. Open the United State article
  2. Tap on the Quick facts and look for the audio files inside it

Actual Results:

Shows incomplete audio player toolbar and not be able to play the audio

Expected Results:

Shows complete audio player toolbar and able to play the audio

Event Timeline

cooltey created this task.Jun 29 2018, 5:15 PM
Restricted Application removed a project: Patch-For-Review. · View Herald TranscriptJun 29 2018, 5:15 PM
bearND added a subscriber: bearND.

This is the same in the corresponding Parsoid HTML when running in Chrome: https://en.wikipedia.org/api/rest_v1/page/html/United_States. (It acts better in FF, but that doesn't help the Android app experience).

It seems like the play button now doesn't appear unless you have 56px of height :(

In T186358, we increased it to 32px, but continuing to track Chrome changes seems futile, so we added a class mw-default-audio-height which can be used to override it.

Dbrant added a subscriber: Dbrant.Jul 16 2018, 7:39 PM

So, it looks like there are a couple of issues at work here:

  • Parsoid seems to emit the audio inside a <video> tag instead of an <audio> tag. @Arlolra do you know why this might be happening?
  • It sets an explicit width and height for the <video> element, which I'm not sure is necessary. If we simply remove the width and height attributes (and make this into an <audio> element), it will allow the browser to render the native playback controls in whatever dimensions it likes. (I can see how explicit dimensions might be useful for video content, but it's definitely problematic for audio).
cooltey removed cooltey as the assignee of this task.Jul 16 2018, 7:53 PM
  • Parsoid seems to emit the audio inside a <video> tag instead of an <audio> tag. @Arlolra do you know why this might be happening?

It's by design, see the spec https://www.mediawiki.org/wiki/Specs/HTML/1.7.0#Audio/Video

  • It sets an explicit width and height for the <video> element, which I'm not sure is necessary. If we simply remove the width and height attributes (and make this into an <audio> element), it will allow the browser to render the native playback controls in whatever dimensions it likes. (I can see how explicit dimensions might be useful for video content, but it's definitely problematic for audio).

This is being discussed in T133673

It's by design, see the spec https://www.mediawiki.org/wiki/Specs/HTML/1.7.0#Audio/Video

I see, thanks! I'm afraid that's somewhat regrettable, since browsers (at least Chrome) seem to apply significantly different styling to <video> versus <audio> components. Specifically, as we have observed, it causes the browser to show video-specific controls when they're not actually applicable to the contents of the <video> tag. These controls are also arranged differently from their audio counterparts.

We can work around this by replacing the <video> tag with an <audio> tag on the client side (based on the mw:Audio type), but would it not be better to produce the correct tag upstream?

Change 446186 had a related patch set uploaded (by Dbrant; owner: Dbrant):
[apps/android/wikipedia@master] For real, fix audio playback in the app.

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

It sets an explicit width and height for the <video> element, which I'm not sure is necessary.

Setting the explicit dimension attributes is so that editors can define these things in wikitext, but still allow for clients to override them with CSS, which is consistent with what we do for images.

(I can see how explicit dimensions might be useful for video content, but it's definitely problematic for audio).

There's an additional selector for the default audio height, so that it can easily be removed where necessary. For example,

.mw-default-audio-height video { height: auto; }

... we have observed, it causes the browser to show video-specific controls when they're not actually applicable to the contents of the <video> tag. These controls are also arranged differently from their audio counterparts.

Which platform were you looking at?

... would it not be better to produce the correct tag upstream?

Please see T133673#2969366 for the rationale for the current choice, but I can reopen this for discussion at the Parsing-Team meeting tomorrow.

Which platform were you looking at?

This is on both desktop and Android versions of Chrome. Looking at [[The Star-Spangled Banner]], for example, here is the audio clip as rendered in a <video> tag, with width and height set to auto:

...and the same clip rendered in an <audio> tag:

The video tag is rendered with some unnecessary and busy-looking UI elements, namely the "speaker" icon image in the background, which is partially obscured by the "play" button, as well as a "full-screen" button which is obviously disabled, but still shown. Whereas the audio tag is rendered perfectly cleanly, with controls that are only relevant for audio playback.

But most importantly, the audio element places the "Play" button to the left of the playback slider, whereas the video tag places it above the playback slider, which makes the Play button inaccessible when we try to constrain the height of the video element.

... would it not be better to produce the correct tag upstream?

Please see T133673#2969366 for the rationale for the current choice, but I can reopen this for discussion at the Parsing-Team meeting tomorrow.

The rationale we used was contingent on "assuming that the HTML5 allows this and does the sensible thing", the latter of which no longer seems to be holding, as you've argued.

So, we agreed to switch to using the audio tag with height and width attributes, which browsers should just ignore. JS can then be used to enforce those constraints, if so desired.

Our deploys are currently blocked on other tasks, so this probably won't make it out until next week. Hopefully that's acceptable.

Our deploys are currently blocked on other tasks, so this probably won't make it out until next week. Hopefully that's acceptable.

This should also be blocked on VE being able to handle the change.

Change 446186 merged by jenkins-bot:
[apps/android/wikipedia@master] For real, fix audio playback in the app.

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

Dbrant claimed this task.Jul 18 2018, 3:45 PM

@ABorbaWMF This is now ready to test with the latest Alpha.

Change 446186 merged by jenkins-bot:
[apps/android/wikipedia@master] For real, fix audio playback in the app.

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

From the change I see that it handles only .ogg files. But this issue seem to be affecting other audio formats too. For example, United_States_military_music_customs#Audio_examples. The third audio is a WAV file.

The following is a screen shot of how it renders in 2.7.237-alpha-2018-07-17


I think Category:Audio_files_by_file_format would give the list of audio files supported by commons.

ssastry moved this task from Needs Triage to Read Views on the Parsoid board.Jul 27 2018, 5:48 AM

Change 449903 had a related patch set uploaded (by Arlolra; owner: Arlolra):
[mediawiki/services/parsoid@master] Use <audio> elements for rendering audio files

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

Change 449903 merged by jenkins-bot:
[mediawiki/services/parsoid@master] Use <audio> elements for rendering audio files

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

Reedy edited projects, added Parsoid-Read-Views; removed Parsoid.Sep 17 2018, 7:25 PM

Mentioned in SAL (#wikimedia-operations) [2018-10-01T21:17:06Z] <arlolra> Updated Parsoid to 224ecde (T198504, T133673, T202666)

Arlolra closed this task as Resolved.Oct 1 2018, 10:13 PM
Aklapper edited projects, added Parsoid; removed Parsoid-Read-Views.Feb 29 2020, 5:14 PM