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

Allow font-variant east-asian shorthand in any position #4945

Merged

Conversation

mdubet
Copy link
Contributor

@mdubet mdubet commented Oct 3, 2022

fd7b349

Allow font-variant east-asian shorthand in any position
https://bugs.webkit.org/show_bug.cgi?id=245972

Reviewed by Darin Adler.

The property font-variant-east-asian can be used in any position
in the font-variant shorthand ; it should not check
that the range is finished.

https://drafts.csswg.org/css-fonts-4/#font-variant-prop

* LayoutTests/fast/text/font-variant-shorthand-expected.txt:
* LayoutTests/fast/text/font-variant-shorthand.html:

Add tests which fail before this change.

* LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-variant-east-asian-invalid-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-variant-east-asian-invalid.html:

Imported from WPT.

* LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-variant-valid-expected.txt:
* Source/WebCore/css/parser/CSSPropertyParser.cpp:
(WebCore::consumeFontVariantEastAsian):

Canonical link: https://commits.webkit.org/255134@main

2123600

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe   πŸ›  πŸ§ͺ win
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim   πŸ›  mac-debug βœ… πŸ›  gtk βœ… πŸ›  wincairo
βœ… πŸ§ͺ webkitperl   πŸ§ͺ ios-wk2   πŸ›  mac-AS-debug βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios   πŸ§ͺ api-mac   πŸ§ͺ api-gtk
βœ… πŸ›  tv   πŸ§ͺ mac-wk1
βœ… πŸ›  tv-sim   πŸ§ͺ mac-wk2
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch   πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  watch-sim   πŸ§ͺ mac-wk2-stress

@mdubet mdubet self-assigned this Oct 3, 2022
@mdubet mdubet added WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit). WebKit Nightly Build labels Oct 3, 2022
@mdubet mdubet force-pushed the font-variant-east-asian-bug-245972 branch from f324eff to efb7837 Compare October 3, 2022 22:16
@mdubet mdubet force-pushed the font-variant-east-asian-bug-245972 branch from efb7837 to c12e3b1 Compare October 3, 2022 22:44
@mdubet mdubet changed the title Allow font-variant east-asian shorthand in any position. Allow font-variant east-asian shorthand in any position Oct 3, 2022
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 4, 2022
Copy link
Member

@nt1m nt1m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if font-variant-east-asian: full-width garbage or font-variant-east-asian: full-width none is correctly handled here. We've had a similar bug with the list-style shorthand, where it supported list-style: none garbage because it didn't do a proper range end check.

Source/WebCore/css/parser/CSSPropertyParser.cpp Outdated Show resolved Hide resolved
Source/WebCore/css/parser/CSSPropertyParser.cpp Outdated Show resolved Hide resolved
@mdubet mdubet removed the merging-blocked Applied to prevent a change from being merged label Oct 4, 2022
@mdubet mdubet force-pushed the font-variant-east-asian-bug-245972 branch from c12e3b1 to 8d4b6dd Compare October 4, 2022 08:02
@mdubet mdubet force-pushed the font-variant-east-asian-bug-245972 branch from 8d4b6dd to 2123600 Compare October 4, 2022 15:53
@nt1m nt1m added the merge-queue Applied to send a pull request to merge-queue label Oct 4, 2022
https://bugs.webkit.org/show_bug.cgi?id=245972

Reviewed by Darin Adler.

The property font-variant-east-asian can be used in any position
in the font-variant shorthand ; it should not check
that the range is finished.

https://drafts.csswg.org/css-fonts-4/#font-variant-prop

* LayoutTests/fast/text/font-variant-shorthand-expected.txt:
* LayoutTests/fast/text/font-variant-shorthand.html:

Add tests which fail before this change.

* LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-variant-east-asian-invalid-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-variant-east-asian-invalid.html:

Imported from WPT.

* LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-variant-valid-expected.txt:
* Source/WebCore/css/parser/CSSPropertyParser.cpp:
(WebCore::consumeFontVariantEastAsian):

Canonical link: https://commits.webkit.org/255134@main
@webkit-commit-queue
Copy link
Collaborator

Committed 255134@main (fd7b349): https://commits.webkit.org/255134@main

Reviewed commits have been landed. Closing PR #4945 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit fd7b349 into WebKit:main Oct 4, 2022
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit).
Projects
None yet
6 participants