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

Align stroke-dasharray CSS property parsing with the specification #7884

Merged

Conversation

cdumez
Copy link
Contributor

@cdumez cdumez commented Dec 20, 2022

36071c5

Align `stroke-dasharray` CSS property parsing with the specification
https://bugs.webkit.org/show_bug.cgi?id=249615

Reviewed by Simon Fraser.

Align `stroke-dasharray` CSS property parsing with the specification:
- https://svgwg.org/svg2-draft/painting.html#StrokeDashing
- https://svgwg.org/svg2-draft/painting.html#DataTypeDasharray

* LayoutTests/imported/w3c/web-platform-tests/css/css-typed-om/the-stylepropertymap/properties/stroke-dasharray-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-typed-om/the-stylepropertymap/properties/stroke-dasharray.html:
Update stroke-dasharray css-typed-on WPT test to match the specification. In
particular, <length> | <percentage> | <number> should be allowed. Negative
values shouldn't be allowed. Also, computed values should be converted to
lengths if numbers were provided.

* LayoutTests/imported/w3c/web-platform-tests/svg/painting/parsing/stroke-dasharray-invalid-expected.txt:
Rebaseline WPT test now that more checks are passing.

* Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:
Update LengthRawKnownTokenTypeNumberConsumer::consume() to stop ignoring the
UnitlessZeroQuirk parameter. Previously, no matter what value the caller was
passing for this parameter, we were allowing unitless zero. Update call sites
to pass UnitlessZeroQuirk::Allow to maintain pre-existing behavior for now.
However, I made sure to pass UnitlessZeroQuirk::Forbid in
consumeStrokeDasharray() since we want to make sure "0" gets parsed as a
<number>, not a <length> (covered by stroke-dasharray-valid.html WPT test).

(WebCore::CSSPropertyParserHelpers::consumeStrokeDasharray):
Fallback to parsing input as a <number> if parsing it as a <length-percentage>,
as per the specification.

* Source/WebCore/css/typedom/CSSUnitValue.cpp:
(WebCore::isValueOutOfRangeForProperty):
Update isValueOutOfRangeForProperty() to properly indicate that stroke-dasharray
doesn't allow negative values.

* Source/WebCore/style/StyleBuilderConverter.h:
(WebCore::Style::BuilderConverter::convertSVGLengthValue):
(WebCore::Style::BuilderConverter::convertSVGLengthVector):
(WebCore::Style::BuilderConverter::convertStrokeDashArray):
Update our style builder converter to:
- Deal with stroke-dasharray values coming from CSS-Typed-OM, which may not have
  been converted to a CSSValueList yet (unlike values coming from the CSS parser).
- Request that numbers get converted to lengths (by using 'px' unit), now that we
  allow numbers at parser level (per the specification).

* Source/WebCore/svg/SVGLengthValue.cpp:
(WebCore::SVGLengthValue::fromCSSPrimitiveValue):
* Source/WebCore/svg/SVGLengthValue.h:

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

92827d6

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

@cdumez cdumez self-assigned this Dec 20, 2022
@cdumez cdumez added the CSS Cascading Style Sheets implementation label Dec 20, 2022
@cdumez cdumez force-pushed the 249615_stroke-dasharray_fixes branch from ff4a1db to 1446f15 Compare December 20, 2022 00:49
@cdumez cdumez force-pushed the 249615_stroke-dasharray_fixes branch from 1446f15 to 92827d6 Compare December 20, 2022 01:46
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 20, 2022
@cdumez cdumez removed the merging-blocked Applied to prevent a change from being merged label Dec 20, 2022
@cdumez cdumez marked this pull request as ready for review December 20, 2022 16:01
@cdumez
Copy link
Contributor Author

cdumez commented Dec 20, 2022

Upstream WPT PR: web-platform-tests/wpt#37603

@cdumez cdumez added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Dec 20, 2022
https://bugs.webkit.org/show_bug.cgi?id=249615

Reviewed by Simon Fraser.

Align `stroke-dasharray` CSS property parsing with the specification:
- https://svgwg.org/svg2-draft/painting.html#StrokeDashing
- https://svgwg.org/svg2-draft/painting.html#DataTypeDasharray

* LayoutTests/imported/w3c/web-platform-tests/css/css-typed-om/the-stylepropertymap/properties/stroke-dasharray-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-typed-om/the-stylepropertymap/properties/stroke-dasharray.html:
Update stroke-dasharray css-typed-on WPT test to match the specification. In
particular, <length> | <percentage> | <number> should be allowed. Negative
values shouldn't be allowed. Also, computed values should be converted to
lengths if numbers were provided.

* LayoutTests/imported/w3c/web-platform-tests/svg/painting/parsing/stroke-dasharray-invalid-expected.txt:
Rebaseline WPT test now that more checks are passing.

* Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:
Update LengthRawKnownTokenTypeNumberConsumer::consume() to stop ignoring the
UnitlessZeroQuirk parameter. Previously, no matter what value the caller was
passing for this parameter, we were allowing unitless zero. Update call sites
to pass UnitlessZeroQuirk::Allow to maintain pre-existing behavior for now.
However, I made sure to pass UnitlessZeroQuirk::Forbid in
consumeStrokeDasharray() since we want to make sure "0" gets parsed as a
<number>, not a <length> (covered by stroke-dasharray-valid.html WPT test).

(WebCore::CSSPropertyParserHelpers::consumeStrokeDasharray):
Fallback to parsing input as a <number> if parsing it as a <length-percentage>,
as per the specification.

* Source/WebCore/css/typedom/CSSUnitValue.cpp:
(WebCore::isValueOutOfRangeForProperty):
Update isValueOutOfRangeForProperty() to properly indicate that stroke-dasharray
doesn't allow negative values.

* Source/WebCore/style/StyleBuilderConverter.h:
(WebCore::Style::BuilderConverter::convertSVGLengthValue):
(WebCore::Style::BuilderConverter::convertSVGLengthVector):
(WebCore::Style::BuilderConverter::convertStrokeDashArray):
Update our style builder converter to:
- Deal with stroke-dasharray values coming from CSS-Typed-OM, which may not have
  been converted to a CSSValueList yet (unlike values coming from the CSS parser).
- Request that numbers get converted to lengths (by using 'px' unit), now that we
  allow numbers at parser level (per the specification).

* Source/WebCore/svg/SVGLengthValue.cpp:
(WebCore::SVGLengthValue::fromCSSPrimitiveValue):
* Source/WebCore/svg/SVGLengthValue.h:

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

Committed 258136@main (36071c5): https://commits.webkit.org/258136@main

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

@webkit-early-warning-system webkit-early-warning-system merged commit 36071c5 into WebKit:main Dec 20, 2022
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Cascading Style Sheets implementation
Projects
None yet
5 participants