1716675 - audio element without control should be display:none even if we set display to other values

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Boris Chiou [:boris]]

For example:

<audio style="width: 100px; height: 100px; background: green; display: block;"> </audio>

We see a 100x100 green box now on Firefox because the display:block overrides the display:none which defined in layout/style/res/html.css. In Chrome (and Safari), they define this with !important [1], so the user cannot override it by non-important rule. Perhaps we should do the similar thing.

[1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/resources/html.css;l=164-166?q=audio:not&ss=chromium%2Fchromium%2Fsrc

Comment 1

[Boris Chiou [:boris]]

Attachment: test.html

Comment 2

[Boris Chiou [:boris]]

Attachment: test.html

Comment 3

[Boris Chiou [:boris]]

Reference: https://html.spec.whatwg.org/multipage/rendering.html#embedded-content-rendering-rules

The audio element, when it is exposing a user interface, is expected to be treated as a replaced element about one line high, as wide as is necessary to expose the user agent's user interface features. When an audio element is not exposing a user interface, the user agent is expected to force its 'display' property to compute to 'none', irrespective of CSS rules.

Comment 4

[Boris Chiou [:boris]]

Attachment: Bug 1716675 - Set 'display:none' with important rule for audio element without controls.

If the audio element doesn't have control, it is not exposing a user
interface, so the user agent is expected to force its display property
to compute to none, irrespective fo CSS rules.

https://html.spec.whatwg.org/multipage/rendering.html#embedded-content-rendering-rules

Comment 5

[Boris Chiou [:boris]]

Attachment: Bug 1716675 - Move Gecko's internal tests of audio with controls into wpt.

Comment 6

[Mats Palmgren (inactive)]

Hmm, I dislike !important in UA sheets in general. It's against web platform design principles for user agent implementors to impose UA declarations that cannot be overridden by authors. We should only do that if we have extremely good reasons to do so, e.g. security/privacy issues or "impossible to implement" etc.

That said, the HTML spec does indeed call for this behavior:

When an audio element is not exposing a user interface, the user agent is expected to force its 'display' property to compute to 'none', irrespective of CSS rules.

https://html.spec.whatwg.org/multipage/rendering.html#embedded-content-rendering-rules

Does anyone know how that formulation came to be in the spec? If the justification for it is weak, then perhaps we should argue for it to be removed and ask other UAs to remove !important in their UA sheets?

Personally, I don't see why <audio> should be special in this regard. We allow display:block on a <video> element in the same situation for example.

Anne, do you happen to know the discussion behind the spec text here?

Comment 7

[Anne (:annevk)]

I found https://github.com/whatwg/html/commit/e6d0ca64f275c83fc094eef544ec5afc3b3a3e80 (I think gow stands for Gecko / Opera / WebKit being impacted, but not sure), but no context unfortunately. There are other things the HTML standard uses !important for so I don't feel too strongly about this, but I also wouldn't be opposed to it being changed if everyone can agree on that.

Comment 8

[Mats Palmgren (inactive)]

There are other things the HTML standard uses !important for ...

In my experience, !important in UA sheets are pretty much always misguided and unnecessary. And this is one example of that.

That said, I don't feel that strongly about this particular case given that it's been in the spec for over a decade...

Comment 9

[Boris Chiou [:boris]]

I see. Thanks for all the information and suggestion. I'd keep this bug for tracking and not land these patches for now.

Comment 10

[Anne (:annevk)]

Well, we should either land this or file an issue against the HTML Standard and reach out to other implementers, right? As long as they do not change anything here, there's a potential for this to result in compatibility issues.

Comment 11

[Mats Palmgren (inactive)]

I agree. We should probably just land it. Given that the spec change happened more than a decade ago, I doubt other vendors would be willing to change this.

Comment 12

[Daniel Holbert [:dholbert]]

ni=boris to proceed here, per recent comments.

Comment 13

[Boris Chiou [:boris]]

I see. Let's just land this.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/041fb25cf007
Set 'display:none' with important rule for audio element without controls. r=emilio
https://hg.mozilla.org/integration/autoland/rev/9c31f03fa2ba
Move Gecko's internal tests of audio with controls into wpt. r=emilio

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/29449 for changes under testing/web-platform/tests

Comment 16

[Marian-Vasile Laza]

Backed out 2 changesets (Bug 1716675) for causing mochitest failures on test_group_touchevents-2.html.
Backout link
Push with failures
Failure Log

Comment 17

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 18

[Boris Chiou [:boris]]

Looks like we have to update the subtests in test_group_touchevents-2.html.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/088b8c8940cd
Set 'display:none' with important rule for audio element without controls. r=emilio,botond
https://hg.mozilla.org/integration/autoland/rev/981d6d4958c4
Move Gecko's internal tests of audio with controls into wpt. r=emilio

Comment 20

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/088b8c8940cd
https://hg.mozilla.org/mozilla-central/rev/981d6d4958c4

Comment 21

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot