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

Revert "Update Safari data for aspect-ratio CSS property" #20031

Conversation

foolip
Copy link
Contributor

@foolip foolip commented Jun 5, 2023

Reverts #19570

@foolip foolip requested a review from queengooborg June 5, 2023 13:19
@github-actions github-actions bot added the data:css Compat data for CSS features. https://developer.mozilla.org/docs/Web/CSS label Jun 5, 2023
@foolip foolip requested a review from Elchi3 June 5, 2023 13:19
@foolip
Copy link
Contributor Author

foolip commented Jun 5, 2023

In #19570, I think the only evidence for the change is https://mdn-bcd-collector.gooborg.com/tests/css/properties/aspect-ratio, and I couldn't confirm any effect of aspect-ratio in Safari 14.1. I've tested <div>, <img> and <video>.

@gsnedders
Copy link
Contributor

This was probably caused by the bug where CSS.supports didn't take settings into account, and would return false-positives.

@foolip
Copy link
Contributor Author

foolip commented Jun 6, 2023

Ah, that would make sense! Would you happen to know how far back that bug goes, and is there a way to list flags? I suspect there are more errors in BCD due to this...

@queengooborg
Copy link
Contributor

queengooborg commented Jun 6, 2023

I'm unable to verify that this property does anything to images in Safari 14.1, though I could have sworn I had tested for support before...

@queengooborg queengooborg merged commit 30eb389 into main Jun 6, 2023
@queengooborg queengooborg deleted the revert-19570-css/properties/aspect-ratio/safari-corrections branch June 6, 2023 08:09
@gsnedders
Copy link
Contributor

Ah, that would make sense! Would you happen to know how far back that bug goes, and is there a way to list flags? I suspect there are more errors in BCD due to this...

Goes back to CSS.supports's introduction, I'm pretty sure, and seems to have been fixed in Safari 15. And then there's also foolip/mdn-bcd-collector#1955.

@queengooborg
Copy link
Contributor

And then there's also foolip/mdn-bcd-collector#1955.

This was actually probably already fixed by openwebdocs/mdn-bcd-collector@7eff721!

@gsnedders
Copy link
Contributor

It might be worthwhile using that code branch for Safari < 15, dunno.

@queengooborg
Copy link
Contributor

It might be worthwhile using that code branch for Safari < 15, dunno.

Oh it's already been in use; that branch is the main deployment of the collector at https://mdn-bcd-collector.gooborg.com! I've also modified the test in openwebdocs/mdn-bcd-collector@55ba62d to pass an applicable value (instead of just inherit), which will be released in v10 of the collector!

@gsnedders
Copy link
Contributor

Did openwebdocs/mdn-bcd-collector@55ba62d cause any changes? I don't think it should for Safari? Maybe it does.

The better fix would just be what I meant above—not using the if ('CSS' in window && window.CSS.supports) branch on Safari < 15.

@queengooborg
Copy link
Contributor

Did openwebdocs/mdn-bcd-collector@55ba62d cause any changes? I don't think it should for Safari? Maybe it does.

It did, yes -- it reports false in Safari 14.1!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:css Compat data for CSS features. https://developer.mozilla.org/docs/Web/CSS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants