Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal to remove <audio> element from contain-inline-size-replaced.html #317

Closed
vmpstr opened this issue Apr 3, 2023 · 5 comments · Fixed by web-platform-tests/wpt#39749
Labels
focus area: Containment test-change-proposal Proposal to add or remove tests for an interop area

Comments

@vmpstr
Copy link

vmpstr commented Apr 3, 2023

Test List

https://wpt.fyi/results/css/css-contain/contain-inline-size-replaced.html

Rationale

I'm proposing to remove element subtests from the contain-inline-size-replaced.html tests, which is a part of containment interop

The element is underspecified and is implemented in different ways in different browsers. It is sized differently and responds to containment in different ways. One of the subtests is failing on 3/3 major browsers. The other subtest is failing on 2/3 major browsers.

Additional discussion: web-platform-tests/wpt#39325

@vmpstr vmpstr added the test-change-proposal Proposal to add or remove tests for an interop area label Apr 3, 2023
@vmpstr
Copy link
Author

vmpstr commented Apr 14, 2023

@dholbert you mentioned that you would be ok with this change, is that still the case?

@nt1m would you be able to say if WebKit is OK with change as well?

If everyone agrees, I can make the change

/cc @foolip as an fyi

@nt1m
Copy link
Member

nt1m commented Apr 14, 2023

@rwlbuis @cathiechen What do you think?

@dholbert
Copy link

dholbert commented Apr 14, 2023

Yes, I'm still OK with the change; my thinking from the "additional discussion" linked-issue still applies.

(tl;dr: the test is assuming that browsers don't have UA stylesheet rules to apply a particular writing-mode or default width/height values on this element. In practice, browsers do actually do those things. And in terms of actual author-impacting interop concerns, the "does contain:size collapse an audio element to 0x0" question does not seem particularly important, given that contain:size is meant to be used on container elements (to contain the size of their arbitrary subtree content), and it's not super useful on leaf nodes.)

@rwlbuis
Copy link

rwlbuis commented Apr 21, 2023

@rwlbuis @cathiechen What do you think?

Cathie knows size containment better. FWIW, at the time audio was the only subtest I could not fix (https://bugs.webkit.org/show_bug.cgi?id=244799), so would be nice to get this corrected and passing.

@cathiechen
Copy link

Sorry for the slow reply!

Yes, this change looks good to me!
In WebKit, audio has min-width: 44px !important, so contain: inline-size is not effective in this case.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 28, 2023
Fixes: web-platform-tests/interop#317

R=foolip@chromium.org, chrishtr@chromium.org

Change-Id: Ia2b14f61c64c600bca23d9eb57ae4e7dbf5d961f
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 28, 2023
Fixes: web-platform-tests/interop#317

R=foolip@chromium.org, chrishtr@chromium.org

Change-Id: Ia2b14f61c64c600bca23d9eb57ae4e7dbf5d961f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4490304
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Vladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1137342}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 22, 2023
…inline-size-replaced test., a=testonly

Automatic update from web-platform-tests
WPT: Remove audio elements from contain-inline-size-replaced test.

Fixes: web-platform-tests/interop#317

R=foolip@chromium.org, chrishtr@chromium.org

Change-Id: Ia2b14f61c64c600bca23d9eb57ae4e7dbf5d961f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4490304
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Vladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1137342}

--

wpt-commits: d9f5e1fca558380dcfabb8e359c8fc2ab1cd3189
wpt-pr: 39749
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue May 23, 2023
…inline-size-replaced test., a=testonly

Automatic update from web-platform-tests
WPT: Remove audio elements from contain-inline-size-replaced test.

Fixes: web-platform-tests/interop#317

R=foolip@chromium.org, chrishtr@chromium.org

Change-Id: Ia2b14f61c64c600bca23d9eb57ae4e7dbf5d961f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4490304
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Vladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1137342}

--

wpt-commits: d9f5e1fca558380dcfabb8e359c8fc2ab1cd3189
wpt-pr: 39749
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus area: Containment test-change-proposal Proposal to add or remove tests for an interop area
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants