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

[css-anchor-position-1] Add parsing support for anchor() #29629

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

RWDavid
Copy link
Contributor

@RWDavid RWDavid commented Jun 7, 2024

258ed7a

[css-anchor-position-1] Add parsing support for `anchor()`
https://bugs.webkit.org/show_bug.cgi?id=275063
rdar://129168625

Reviewed by Antti Koivisto.

This patch adds initial parsing support for `anchor()` and introduces
a new CSSValue subclass named CSSAnchorValue. This class is also
implemented as a CSSPrimitiveValue. Note that the parsers for inset
properties (left, top, inset-block-end, etc.) have been modified to
accept `anchor()` as a value, whereas `anchor()` is not accepted as a
<length> in other properties by default.

This patch does not support parsing `anchor()` calls in other CSS math
functions (such as `calc()` and `min()`).

Previously WebKit ignored inset property declarations using `anchor()`.
Now, because these values are properly parsed, any inset properties
using `anchor()` are assigned a default fixed length of 0 during
style resolution. (This causes many spurious test rebaselines.)

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/css/css-anchor-position/
* LayoutTests/imported/w3c/web-platform-tests/html/dom/elements/global-attributes/the-anchor-attribute-003.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-anchor-display-none.tentative-expected.txt:
* Source/WebCore/Headers.cmake:
* Source/WebCore/Sources.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/css/CSSAnchorValue.cpp: Added.
(WebCore::CSSAnchorValue::create):
(WebCore::CSSAnchorValue::collectComputedStyleDependencies const):
(WebCore::CSSAnchorValue::customCSSText const):
(WebCore::CSSAnchorValue::equals const):
* Source/WebCore/css/CSSAnchorValue.h:
* Source/WebCore/css/CSSPrimitiveValue.cpp:
(WebCore::isValidCSSUnitTypeForDoubleConversion):
(WebCore::isStringType):
(WebCore::CSSPrimitiveValue::CSSPrimitiveValue):
(WebCore::CSSPrimitiveValue::~CSSPrimitiveValue):
(WebCore::CSSPrimitiveValue::create):
(WebCore::CSSPrimitiveValue::unitTypeString):
(WebCore::CSSPrimitiveValue::serializeInternal const):
(WebCore::CSSPrimitiveValue::equals const):
(WebCore::CSSPrimitiveValue::addDerivedHash const):
(WebCore::CSSPrimitiveValue::collectComputedStyleDependencies const):
* Source/WebCore/css/CSSPrimitiveValue.h:
* Source/WebCore/css/CSSProperties.json:
* Source/WebCore/css/CSSUnits.cpp:
(WebCore::unitCategory):
(WebCore::operator<<):
* Source/WebCore/css/CSSUnits.h:
* Source/WebCore/css/CSSValue.cpp:
(WebCore::CSSValue::visitDerived):
* Source/WebCore/css/CSSValue.h:
(WebCore::CSSValue::isAnchorValue const):
* Source/WebCore/css/CSSValueKeywords.in:
* Source/WebCore/css/calc/CSSCalcCategoryMapping.cpp:
(WebCore::hasDoubleValue):
* Source/WebCore/css/parser/CSSPropertyParserConsumer+Length.cpp:
(WebCore::CSSPropertyParserHelpers::consumeLengthOrPercent):
* Source/WebCore/css/parser/CSSPropertyParserConsumer+Length.h:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+Primitives.h:
* Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:
(WebCore::CSSPropertyParserHelpers::consumeDashedIdent):
(WebCore::CSSPropertyParserHelpers::consumeAutoOrLengthOrPercent):
(WebCore::CSSPropertyParserHelpers::consumeSide):
(WebCore::CSSPropertyParserHelpers::consumeInsetLogicalStartEnd):
(WebCore::CSSPropertyParserHelpers::consumeAnchor):
* Source/WebCore/css/parser/CSSPropertyParserHelpers.h:
* Source/WebCore/style/StyleBuilderConverter.h:
(WebCore::Style::BuilderConverter::convertLength):

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

ee9c97a

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

@RWDavid RWDavid self-assigned this Jun 7, 2024
@RWDavid RWDavid added the CSS Cascading Style Sheets implementation label Jun 7, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 7, 2024
@RWDavid RWDavid removed the merging-blocked Applied to prevent a change from being merged label Jun 7, 2024
@RWDavid RWDavid force-pushed the eng/parse-anchor-function branch from ea3e65b to 44d3f95 Compare June 7, 2024 19:02
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 7, 2024
@RWDavid RWDavid removed the merging-blocked Applied to prevent a change from being merged label Jun 7, 2024
@RWDavid RWDavid force-pushed the eng/parse-anchor-function branch from 44d3f95 to b27f26c Compare June 7, 2024 21:53
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 7, 2024
Source/WebCore/css/CSSAnchorValue.cpp Outdated Show resolved Hide resolved
Source/WebCore/css/CSSAnchorValue.cpp Outdated Show resolved Hide resolved
Source/WebCore/css/CSSAnchorValue.h Outdated Show resolved Hide resolved
if (!args.size()) {
range = rangeCopy;
auto anchor = CSSAnchorValue::create(WTFMove(anchorElement), WTFMove(anchorSide), nullptr);
return CSSPrimitiveValue::create(anchor);
Copy link
Member

Choose a reason for hiding this comment

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

I don’t get why we wrap CSSAnchorValue inside a CSSPrimitiveValue.

Copy link
Contributor

@anttijk anttijk Jun 10, 2024

Choose a reason for hiding this comment

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

I suggested just following calc() pattern (which works like this) as deviating from it may result in a large refactoring. There are assumptions that only CSSPrimitiveValues map to Length. But maybe those would be easy enough to untangle?

Copy link
Contributor

Choose a reason for hiding this comment

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

That sort of cleanup should also be done for calc() and might be better done separately for both.

@RWDavid RWDavid removed the merging-blocked Applied to prevent a change from being merged label Jun 10, 2024
@nt1m nt1m added the merge-queue Applied to send a pull request to merge-queue label Jun 10, 2024
https://bugs.webkit.org/show_bug.cgi?id=275063
rdar://129168625

Reviewed by Antti Koivisto.

This patch adds initial parsing support for `anchor()` and introduces
a new CSSValue subclass named CSSAnchorValue. This class is also
implemented as a CSSPrimitiveValue. Note that the parsers for inset
properties (left, top, inset-block-end, etc.) have been modified to
accept `anchor()` as a value, whereas `anchor()` is not accepted as a
<length> in other properties by default.

This patch does not support parsing `anchor()` calls in other CSS math
functions (such as `calc()` and `min()`).

Previously WebKit ignored inset property declarations using `anchor()`.
Now, because these values are properly parsed, any inset properties
using `anchor()` are assigned a default fixed length of 0 during
style resolution. (This causes many spurious test rebaselines.)

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/css/css-anchor-position/
* LayoutTests/imported/w3c/web-platform-tests/html/dom/elements/global-attributes/the-anchor-attribute-003.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-anchor-display-none.tentative-expected.txt:
* Source/WebCore/Headers.cmake:
* Source/WebCore/Sources.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/css/CSSAnchorValue.cpp: Added.
(WebCore::CSSAnchorValue::create):
(WebCore::CSSAnchorValue::collectComputedStyleDependencies const):
(WebCore::CSSAnchorValue::customCSSText const):
(WebCore::CSSAnchorValue::equals const):
* Source/WebCore/css/CSSAnchorValue.h:
* Source/WebCore/css/CSSPrimitiveValue.cpp:
(WebCore::isValidCSSUnitTypeForDoubleConversion):
(WebCore::isStringType):
(WebCore::CSSPrimitiveValue::CSSPrimitiveValue):
(WebCore::CSSPrimitiveValue::~CSSPrimitiveValue):
(WebCore::CSSPrimitiveValue::create):
(WebCore::CSSPrimitiveValue::unitTypeString):
(WebCore::CSSPrimitiveValue::serializeInternal const):
(WebCore::CSSPrimitiveValue::equals const):
(WebCore::CSSPrimitiveValue::addDerivedHash const):
(WebCore::CSSPrimitiveValue::collectComputedStyleDependencies const):
* Source/WebCore/css/CSSPrimitiveValue.h:
* Source/WebCore/css/CSSProperties.json:
* Source/WebCore/css/CSSUnits.cpp:
(WebCore::unitCategory):
(WebCore::operator<<):
* Source/WebCore/css/CSSUnits.h:
* Source/WebCore/css/CSSValue.cpp:
(WebCore::CSSValue::visitDerived):
* Source/WebCore/css/CSSValue.h:
(WebCore::CSSValue::isAnchorValue const):
* Source/WebCore/css/CSSValueKeywords.in:
* Source/WebCore/css/calc/CSSCalcCategoryMapping.cpp:
(WebCore::hasDoubleValue):
* Source/WebCore/css/parser/CSSPropertyParserConsumer+Length.cpp:
(WebCore::CSSPropertyParserHelpers::consumeLengthOrPercent):
* Source/WebCore/css/parser/CSSPropertyParserConsumer+Length.h:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+Primitives.h:
* Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:
(WebCore::CSSPropertyParserHelpers::consumeDashedIdent):
(WebCore::CSSPropertyParserHelpers::consumeAutoOrLengthOrPercent):
(WebCore::CSSPropertyParserHelpers::consumeSide):
(WebCore::CSSPropertyParserHelpers::consumeInsetLogicalStartEnd):
(WebCore::CSSPropertyParserHelpers::consumeAnchor):
* Source/WebCore/css/parser/CSSPropertyParserHelpers.h:
* Source/WebCore/style/StyleBuilderConverter.h:
(WebCore::Style::BuilderConverter::convertLength):

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

Committed 279891@main (258ed7a): https://commits.webkit.org/279891@main

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

@webkit-commit-queue webkit-commit-queue merged commit 258ed7a into WebKit:main Jun 10, 2024
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 10, 2024
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
7 participants