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

Compat 2021 carryover #2

Closed
chrishtr opened this issue Oct 29, 2021 · 26 comments
Closed

Compat 2021 carryover #2

chrishtr opened this issue Oct 29, 2021 · 26 comments
Labels
accepted An accepted proposal focus-area-proposal Focus Area Proposal requires:tests Requires additional tests to be written
Milestone

Comments

@chrishtr
Copy link
Contributor

Compat2021 is the predecessor to Interop2022. It defined 5 key areas where interop was not good enough between browsers, as backed up by strong evidence from developer surveys. All five areas are fully supported features from all browser engines.

Over the course of 2021, there has been excellent progress in the score for all three browser rendering engines. All three are above 90% at this point for their tip-of-tree implementations, whereas at the beginning of the year none were. (Note: the Safari score on the current website is not accurate, due to a technical complication; the actual score is about 92).

I would like to propose that the Compat2021 areas be included as part of the Interop2022 score. This will reinforce that we want interop to continue improving and be sustained for "last year's" metric, not just the new hotness.

@fantasai
Copy link

fantasai commented Nov 3, 2021

Strong +1. In particular, I think we still have work to do on Grid and Flexbox layout, and I wouldn't be surprised if there are some bugs and issues around aspect-ratio or postion:sticky, so it would be good to keep them at a high priority.

@gsnedders gsnedders mentioned this issue Nov 8, 2021
@jgraham
Copy link
Contributor

jgraham commented Nov 11, 2021

I think there's some overlap with #9 in that they are both looking for ways to include ongoing improvements to existing features rather than just focusing on implementing new functionality. I fully agree that this kind of priority is a good idea. However I don't think we should blindly roll over the existing tests/areas, for two reasons:

  • For Compat 2021 in particular there was less explicit buy-in on the focus areas. I think going forward everything should be held to the same standard. Obviously this wouldn't be such a concern in future iterations.
  • It makes sense to reassess the set of tests + etc. to ensure that any remaining failure are things that we think are likely to affect authors, and to ensure that we aren't missing any important cases that weren't included in Compat 2021 due to lack of coverage (i.e. where new tests have been written since the original list was picked).

@foolip
Copy link
Member

foolip commented Nov 11, 2021

Vetting the set of tests for Compat 2021 would be a good idea I think.

I like the model of including all tests in the latest metric, since it means there's just one metric at a time to focus on, but a downside is that over time, the new tests would account for less and less of the overall score. One could mitigate this by weighting new tests to be at least 50%, but that would also make the metric a bit harder to understand.

@foolip foolip mentioned this issue Nov 12, 2021
36 tasks
@foolip foolip added the requires:tests Requires additional tests to be written label Nov 18, 2021
@foolip
Copy link
Member

foolip commented Nov 18, 2021

Added label to mean that we should revisit the tests, not that we don't know what they are.

@foolip
Copy link
Member

foolip commented Nov 22, 2021

It will soon be easier to review the list of tests for Compat 2021. web-platform-tests/wpt-metadata#2196 is the first PR labeling some tests, using "interop-2021" as the label, and once that works I'll label all of them. Then it will be easy to filter out just the tests that are failing in some browser for review.

@foolip
Copy link
Member

foolip commented Nov 24, 2021

The labeling support has landed and I've added the interop-2021 labels. Here are all the tests from Compat 2021, and another view filtered to failing (non-passing) tests:

@jensimmons
Copy link
Contributor

Digging into the tests, it’s interesting to see that Compat 2021 included both:

  • the new CSS property, aspect-ratio
  • the new sizing calculations based on using width & height HTML attributes to calculate an aspect ratio to use in first paint

Those are two very different things. Only the first one was written up in the publicity about this project. But there are tests for both.

Interesting.

@gsnedders
Copy link
Member

Also would be interesting to review would be what tests were excluded (modulo obvious cases, like subgrid and masonry for grid).

That said, there's probably an argument that if we want to do it as carry-over (in part to avoid massively regression scores of existing browsers), then we should just keep the same sets.

@foolip foolip reopened this Nov 30, 2021
@foolip
Copy link
Member

foolip commented Nov 30, 2021

@jensimmons aspect-ratio and the new behavior of width and height attributes are indeed not the same thing, but the latter is defined in terms of the former and tested using it as well. Having both tools available and interoperable seemed like a good idea, so we included the attributes under the aspect-ratio banner.

We can of course revisit this as part of this issue, if some of the tests aren't good.

@foolip
Copy link
Member

foolip commented Nov 30, 2021

In Ecosystem-Infra/wpt-results-analysis#66 @dholbert found an excluded a Houdini-dependent transforms test. I've merged the PR to retroactively exclude it, but we should consider including this test instead:
https://wpt.fyi/results/css/css-transforms/animation/transform-interpolation-inline-value.html

@foolip
Copy link
Member

foolip commented Nov 30, 2021

Thanks to that I also noticed there are some tentative tests included in Compat 2021. Quick analysis:

test results
/css/css-flexbox/table-as-item-min-content-height-1.tentative.html all pass
/css/css-flexbox/table-as-item-min-content-height-2.tentative.html all pass
/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-003.tentative.html all pass
/css/css-position/sticky/position-sticky-large-top-2.tentative.html all pass
/css/css-transforms/backface-visibility-hidden-002.tentative.html Safari fails
/css/css-transforms/backface-visibility-hidden-003.tentative.html Safari fails
/css/css-transforms/backface-visibility-hidden-004.tentative.html Chrome+Firefox fail
/css/css-transforms/backface-visibility-hidden-005.tentative.html Chrome+Firefox fail
/html/rendering/.../img-aspect-ratio-lazy.tentative.html all pass

Whether passing or failing, I think next steps for these tests should be to look into why they're tentative, and if the they can be renamed yet. In cases where they still have to be tentative, we should remove them from the Compat 2021 set.

I've prepared Ecosystem-Infra/wpt-results-analysis#67 + web-platform-tests/wpt-metadata#2218 to drop them, but want to look closer first.

@foolip
Copy link
Member

foolip commented Nov 30, 2021

The Flexbox tentative tests came from https://chromium-review.googlesource.com/c/chromium/src/+/2810520 with this comment:

There is no spec requiring this in the block direction, even though Firefox and legacy Blink both do it, and there is a spec requirement for the inline direction. So the tests are marked tentative.

The Grid test

@davidsgrogan do you know if there's an open spec issue for this? I've searched https://github.com/w3c/csswg-drafts/issues but am unsure if any are related.


The Grid test came from https://chromium-review.googlesource.com/c/chromium/src/+/2523743 and links to w3c/csswg-drafts#5713. That's resolved now, so probably that test can be renamed. @bfgeek can you confirm?


The position: sticky test came from https://chromium-review.googlesource.com/c/chromium/src/+/2528572 and links to w3c/csswg-drafts#2794 which remains open. I've asked a naive question on the issue.


The transforms tests came from https://chromium-review.googlesource.com/c/chromium/src/+/2197895, but I can't find any links or comments explaining why they're tentative. @chrishtr can you help look into these, is there a spec issue for this?


Finally, img-aspect-ratio-lazy.tentative.html came from https://phabricator.services.mozilla.com/D66257 but I can't see anything explaining why it's tentative. @emilio can you help look into this one?

@gsnedders
Copy link
Member

I don't have access to that.

@emilio
Copy link

emilio commented Nov 30, 2021

Finally, img-aspect-ratio-lazy.tentative.html came from https://phabricator.services.mozilla.com/D66257 but I can't see anything explaining why it's tentative. @emilio can you help look into this one?

I think I made it tentative because I didn't find the relevant spec text at the time. I guess the "the user agent does not expect this to change" bits in https://html.spec.whatwg.org/multipage/rendering.html#images-3 sorta cover this case, but the spec doesn't say what happens when none of these conditions happen (presumably you go to the replaced element rendering).

But yeah the test should probably not be tentative, and we should fix the spec to be clear.

@foolip
Copy link
Member

foolip commented Nov 30, 2021

I don't have access to that.

How about this? It looks like the resourcekey bit of the URL is sometimes required lately, but I haven't figured out when that is.

@foolip
Copy link
Member

foolip commented Dec 1, 2021

Two more tests I think we should reconsider are print reftests, which Edge and Safari don't run at all:
https://wpt.fyi/results/css/css-flexbox?q=print&run_id=5763010778890240&run_id=5717707447074816&run_id=5674576043311104&run_id=5652492663652352

@gsnedders is print reftest support for Safari something you think might materialize in 2022? If not, I think we should drop these tests, WDYT?

@foolip
Copy link
Member

foolip commented Dec 1, 2021

Finally, img-aspect-ratio-lazy.tentative.html came from https://phabricator.services.mozilla.com/D66257 but I can't see anything explaining why it's tentative. @emilio can you help look into this one?

I think I made it tentative because I didn't find the relevant spec text at the time. I guess the "the user agent does not expect this to change" bits in https://html.spec.whatwg.org/multipage/rendering.html#images-3 sorta cover this case, but the spec doesn't say what happens when none of these conditions happen (presumably you go to the replaced element rendering).

But yeah the test should probably not be tentative, and we should fix the spec to be clear.

Thanks for looking into this, @emilio! I've sent web-platform-tests/wpt#31805 to rename the test and requested your review.

@gsnedders
Copy link
Member

@gsnedders is print reftest support for Safari something you think might materialize in 2022? If not, I think we should drop these tests, WDYT?

Apple does not comment on future products or releases.

@foolip
Copy link
Member

foolip commented Dec 1, 2021

@gsnedders 😄

I take that to mean it requires some changes to Safari or safaridriver, I wasn't sure if it might be a matter of wptrunner bits only.

So, if anyone, for any reason or no reason, wants to drop print reftests from the test list, I think we should do that.

@foolip
Copy link
Member

foolip commented Dec 2, 2021

Ecosystem-Infra/wpt-results-analysis#69 is another case for our consideration, where @stephenmcgruer discovered that two tests pass in Chrome Dev only because of --experimental-web-platform-features and that behavior isn't on its way to stable. I'd default to dropping those tests, but it would be good to get the input of other browser vendors on the behavior in question.

@foolip foolip changed the title Compat2021 carryover Compat 2021 carryover Dec 3, 2021
@gsnedders
Copy link
Member

We should probably at least triage https://wpt.fyi/results/?label=master&label=stable&product=chrome&product=firefox&product=safari&aligned&q=label%3Ainterop-2021%20count%3A3%28status%3A%21pass%26status%3A%21ok%29 (i.e., tests that fail in stable releases of all three major engines).

@foolip foolip added the accepted An accepted proposal label Dec 14, 2021
@foolip
Copy link
Member

foolip commented Jan 20, 2022

Given no pushback on #2 (comment), I have dropped the print reftests in Ecosystem-Infra/wpt-results-analysis@f805a0e + web-platform-tests/wpt-metadata#2403.

@foolip
Copy link
Member

foolip commented Jan 20, 2022

@foolip
Copy link
Member

foolip commented Jan 20, 2022

One Grid test from Interop 2021 came up in #48:

/css/css-grid/grid-definition/grid-limits-001.html is TIMEOUT in Safari. I've tried running it locally and it just shows a white page, but no console errors.

I would guess the test is OK since it passes in Chrome and Firefox, but this might be worth looking into for someone on the WebKit team, just in case it's a bad test that needs fixing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted An accepted proposal focus-area-proposal Focus Area Proposal requires:tests Requires additional tests to be written
Projects
None yet
Development

No branches or pull requests

7 participants