Page MenuHomePhabricator

Add width/height attributes to the <audio><video> tag
Open, MediumPublic

Description

Reasons:

  1. It's consistent with how we output images
  2. CSS can override attributes, but not style="" statements.

Details

Related Gerrit Patches:

Related Objects

Event Timeline

TheDJ created this task.Apr 26 2016, 12:01 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 26 2016, 12:01 PM

Change 262989 had a related patch set uploaded (by TheDJ):
Output width and height attribs for <video>

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

TheDJ changed the task status from Open to Stalled.Jan 25 2017, 4:16 PM

To me, this is stuck on the issue of <audio> and <video> tags and dimensions and what this will mean for Parsoid and VE. By default, an <audio> has an inherent size, but no width and height attributes (because it is not a "replaced element") . This means <audio> requires CSS to specify it's dimensions, and <video> can use CSS and/or attributes.

This really complicates things for Parsoid and VE, since our audio players have configurable width (but a fixed height). We will we have disjunct settings for dimensions between audio/video files, or unify it and just use CSS ? Or use both ?

cscott added a subscriber: cscott.Jan 25 2017, 5:00 PM

I thought you had a suggestion just to use <video> for both at one point?

From my perspective, the goal is that Parsoid output should display "pretty close to correct" on a modern HTML5 browser with no special JS (that is, wikipedia-specific options may be missing, but all media settings that *can* be represented in standard HTML5 are), and should be "readable" with no special CSS -- that is, we already implement the "float left/right" and other image properties via specific classes applied to images, but the images don't *disappear* or not render if you don't load the MW CSS, they just aren't placed correctly.

So, trying to apply that general rule to the <audio>/<video> question, one option is to use actual width and height attributes on <audio>, which HTML5 ought to just ignore. This is "readable" w/o CSS, but it doesn't display "pretty close to correct" on pure HTML5 -- you'd need some JS thunk to transfer the width and height attributes to actual CSS width/height.

So using a <video> element for audio -- assuming that the HTML5 allows this and does the sensible thing -- seems preferable, since then you get correct height/width of the player in standard HTML5 without requiring any special JS magic. Sure, you give up a little bit of semantic information, but you could recapture that by looking at the media type of the embedded file.

cscott added a comment.Mar 9 2017, 6:25 PM

In discussions today, we decided to go with <video> for everything, including audio, and use typeof="mw:Audio" to indicate audio files (since the viewer displays slightly different UI for audio vs video).

Change 349973 had a related patch set uploaded (by Arlolra):
[mediawiki/services/parsoid@master] T133673: Set default dimensions for audio files

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

Change 349973 merged by jenkins-bot:
[mediawiki/services/parsoid@master] T133673: Set default dimensions for audio files

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

Arlolra changed the task status from Stalled to Open.Apr 26 2017, 12:49 PM
Arlolra added a subscriber: Arlolra.

This really complicates things for Parsoid and VE, since our audio players have configurable width (but a fixed height).

In the patch above, we went with a fixed height of 20px, which seems to match TimedMediaHandler, but @cscott was arguing that audio content should respect a defined height. Any thoughts on that?

Change 262989 abandoned by TheDJ:
Output width and height attribs for <video>

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

TheDJ removed TheDJ as the assignee of this task.Sep 18 2017, 1:40 PM

In the patch above, we went with a fixed height of 20px, which seems to match TimedMediaHandler, but @cscott was arguing that audio content should respect a defined height. Any thoughts on that?

Being bumped to 32px in T186358. Still no decision on respecting a defined height.

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

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

Change 505644 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] Synchronize allowed attributes for <audio> with Parsoid/TimedMediaHandler

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

Change 505644 merged by jenkins-bot:
[mediawiki/core@master] Synchronize allowed attributes for <audio> with Parsoid/TimedMediaHandler

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