border-image-* returns the initial keyword instead of the initial value when setting border-image shorthand #6652

darinadler · 2022-11-18T22:33:09Z

`c0c6ad1`

border-image-* returns the initial keyword instead of the initial value when setting border-image shorthand
https://bugs.webkit.org/show_bug.cgi?id=244657
rdar://problem/99420050

Reviewed by Sam Weinig.

* LayoutTests/fast/borders/border-image-legacy.html: Expect initial values instead of the
keyword "initial" for specified style.

* LayoutTests/imported/w3c/web-platform-tests/css/css-backgrounds/parsing/border-image-shorthand.sub-expected.txt:
Expect PASS.
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/css/css-backgrounds/parsing/border-image-shorthand.sub-expected.txt:
Ditto.

* Source/WebCore/css/StyleProperties.cpp:
(WebCore::StyleProperties::borderImagePropertyValue const): Change the code that checks for
implicit values to not require use of the initial keyword.

* Source/WebCore/css/parser/CSSPropertyParser.cpp:
(WebCore::CSSPropertyParser::consumeBorderImage): Set initial values using the actual values
rather than the initial keyword.

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

a2d03e8

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

webkit-early-warning-system · 2022-11-18T22:33:15Z

EWS run on previous version of this PR (hash f5b4d1d)

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
⏳ 🛠 🧪 jsc	✅ 🛠 tv	~~🧪 mac-wk1~~	⏳ 🛠 jsc-armv7
	✅ 🛠 tv-sim	~~🧪 mac-wk2~~	⏳ 🧪 jsc-armv7-tests
	✅ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch-sim	~~🧪 mac-wk2-stress~~

webkit-early-warning-system · 2022-11-18T22:33:53Z

EWS run on previous version of this PR (hash f5b4d1d)

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
⏳ 🛠 🧪 jsc	⏳ 🛠 tv	⏳ 🧪 mac-wk1	⏳ 🛠 jsc-armv7
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2	⏳ 🧪 jsc-armv7-tests
	⏳ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2	⏳ 🛠 jsc-mips
	⏳ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress	⏳ 🧪 jsc-mips-tests

webkit-early-warning-system · 2022-11-18T22:40:48Z

EWS run on previous version of this PR (hash 59295d1)

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
⏳ 🛠 🧪 jsc	⏳ 🛠 tv	⏳ 🧪 mac-wk1	⏳ 🛠 jsc-armv7
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2	⏳ 🧪 jsc-armv7-tests
	⏳ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2	⏳ 🛠 jsc-mips
	⏳ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress	⏳ 🧪 jsc-mips-tests

webkit-early-warning-system · 2022-11-18T22:42:19Z

EWS run on previous version of this PR (hash 59295d1)

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~~
	✅ 🛠 watch	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

Loirooriol

Informal LGTM.

Source/WebCore/css/parser/CSSPropertyParser.cpp

Loirooriol

I think you forgot to upload the new version of the patch

Source/WebCore/css/parser/CSSPropertyParser.cpp

darinadler · 2022-11-19T02:20:11Z

I think you forgot to upload the new version of the patch

It’s coming, still testing it.

webkit-early-warning-system · 2022-11-19T02:22:26Z

EWS run on current version of this PR (hash c3854b1)

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
⏳ 🛠 🧪 jsc	⏳ 🛠 tv	⏳ 🧪 mac-wk1	⏳ 🛠 jsc-armv7
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2	⏳ 🧪 jsc-armv7-tests
	⏳ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2	⏳ 🛠 jsc-mips
	⏳ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress	⏳ 🧪 jsc-mips-tests

webkit-early-warning-system · 2022-11-19T02:22:27Z

EWS run on previous version of this PR (hash c3854b1)

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

darinadler · 2022-11-19T12:17:19Z

I do not understand these compilation errors that we are seeing on multiple platforms. It says the types CSSBorderImageSliceValue and CSSBorderImageWidthValue are incomplete when trying to compile the new code in CSSPropertyParser.cpp but:

Those types are already used elsewhere in CSSPropertyParser.cpp, in the same way, calling their create functions.
The headers for both of those types are included at the top of CSSPropertyParser.cpp.

Loirooriol · 2022-11-19T15:52:04Z

#6592 just moved the headers into CSSPropertyParserHelpers.cpp

weinig · 2022-11-20T02:02:31Z

I do not understand these compilation errors that we are seeing on multiple platforms. It says the types CSSBorderImageSliceValue and CSSBorderImageWidthValue are incomplete when trying to compile the new code in CSSPropertyParser.cpp but:

Those types are already used elsewhere in CSSPropertyParser.cpp, in the same way, calling their create functions.

The headers for both of those types are included at the top of CSSPropertyParser.cpp.

Pretty sure it merged in my change and now the #includes are a bit different.

webkit-early-warning-system · 2022-11-28T23:42:19Z

EWS run on current version of this PR (hash a2d03e8)

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

Loirooriol · 2022-11-29T00:52:24Z

Source/WebCore/css/parser/CSSPropertyParser.cpp

@@ -30,6 +30,8 @@
 #include "config.h"
 #include "CSSPropertyParser.h"

+#include "CSSBorderImageSliceValue.h"
+#include "CSSBorderImageWidthValue.h"


consumeBorderImageComponents is in CSSPropertyParserHelpers.cpp already, so you could move consumeBorderImage too and then no need to include the headers in both places.

I’d hate to delay landing this small change further for that, but I guess I could do it. At the time I started this change the file already included those headers.

Not blocking on this

…ue when setting border-image shorthand https://bugs.webkit.org/show_bug.cgi?id=244657 rdar://problem/99420050 Reviewed by Sam Weinig. * LayoutTests/fast/borders/border-image-legacy.html: Expect initial values instead of the keyword "initial" for specified style. * LayoutTests/imported/w3c/web-platform-tests/css/css-backgrounds/parsing/border-image-shorthand.sub-expected.txt: Expect PASS. * LayoutTests/platform/glib/imported/w3c/web-platform-tests/css/css-backgrounds/parsing/border-image-shorthand.sub-expected.txt: Ditto. * Source/WebCore/css/StyleProperties.cpp: (WebCore::StyleProperties::borderImagePropertyValue const): Change the code that checks for implicit values to not require use of the initial keyword. * Source/WebCore/css/parser/CSSPropertyParser.cpp: (WebCore::CSSPropertyParser::consumeBorderImage): Set initial values using the actual values rather than the initial keyword. Canonical link: https://commits.webkit.org/257119@main

webkit-commit-queue · 2022-11-29T16:12:37Z

Committed 257119@main (c0c6ad1): https://commits.webkit.org/257119@main

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

darinadler self-assigned this Nov 18, 2022

darinadler added the CSS Cascading Style Sheets implementation label Nov 18, 2022

darinadler requested review from nt1m, weinig and Loirooriol November 18, 2022 22:33

darinadler force-pushed the eng/border-image--returns-the-initial-keyword-instead-of-the-initial-value-when-setting-border-image-shorthand branch from f5b4d1d to 59295d1 Compare November 18, 2022 22:40

weinig approved these changes Nov 18, 2022

View reviewed changes

Loirooriol approved these changes Nov 18, 2022

View reviewed changes

Source/WebCore/css/parser/CSSPropertyParser.cpp Outdated Show resolved Hide resolved

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 19, 2022

nt1m reviewed Nov 19, 2022

View reviewed changes

Source/WebCore/css/parser/CSSPropertyParser.cpp Outdated Show resolved Hide resolved

Loirooriol requested changes Nov 19, 2022

View reviewed changes

Source/WebCore/css/parser/CSSPropertyParser.cpp Outdated Show resolved Hide resolved

Loirooriol reviewed Nov 19, 2022

View reviewed changes

Source/WebCore/css/parser/CSSPropertyParser.cpp Outdated Show resolved Hide resolved

Source/WebCore/css/parser/CSSPropertyParser.cpp Outdated Show resolved Hide resolved

darinadler removed the merging-blocked Applied to prevent a change from being merged label Nov 19, 2022

darinadler force-pushed the eng/border-image--returns-the-initial-keyword-instead-of-the-initial-value-when-setting-border-image-shorthand branch from 59295d1 to c3854b1 Compare November 19, 2022 02:22

Loirooriol approved these changes Nov 19, 2022

View reviewed changes

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 19, 2022

darinadler added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Nov 19, 2022

webkit-ews-buildbot added merging-blocked Applied to prevent a change from being merged and removed merge-queue Applied to send a pull request to merge-queue labels Nov 19, 2022

darinadler removed the merging-blocked Applied to prevent a change from being merged label Nov 28, 2022

darinadler force-pushed the eng/border-image--returns-the-initial-keyword-instead-of-the-initial-value-when-setting-border-image-shorthand branch from c3854b1 to a2d03e8 Compare November 28, 2022 23:39

Loirooriol reviewed Nov 29, 2022

View reviewed changes

darinadler added the merge-queue Applied to send a pull request to merge-queue label Nov 29, 2022

webkit-commit-queue force-pushed the eng/border-image--returns-the-initial-keyword-instead-of-the-initial-value-when-setting-border-image-shorthand branch from a2d03e8 to 03e94a8 Compare November 29, 2022 16:10

webkit-commit-queue force-pushed the eng/border-image--returns-the-initial-keyword-instead-of-the-initial-value-when-setting-border-image-shorthand branch from 03e94a8 to c0c6ad1 Compare November 29, 2022 16:12

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Nov 29, 2022

webkit-commit-queue merged commit c0c6ad1 into WebKit:main Nov 29, 2022

darinadler deleted the eng/border-image--returns-the-initial-keyword-instead-of-the-initial-value-when-setting-border-image-shorthand branch November 29, 2022 17:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

border-image-* returns the initial keyword instead of the initial value when setting border-image shorthand #6652

border-image-* returns the initial keyword instead of the initial value when setting border-image shorthand #6652

darinadler commented Nov 18, 2022 •

edited by webkit-early-warning-system

webkit-early-warning-system commented Nov 18, 2022 •

edited

webkit-early-warning-system commented Nov 18, 2022 •

edited

webkit-early-warning-system commented Nov 18, 2022 •

edited

webkit-early-warning-system commented Nov 18, 2022 •

edited

Loirooriol left a comment

Loirooriol left a comment

darinadler commented Nov 19, 2022

webkit-early-warning-system commented Nov 19, 2022 •

edited

webkit-early-warning-system commented Nov 19, 2022 •

edited

darinadler commented Nov 19, 2022

Loirooriol commented Nov 19, 2022

weinig commented Nov 20, 2022

webkit-early-warning-system commented Nov 28, 2022 •

edited

Loirooriol Nov 29, 2022

darinadler Nov 29, 2022 •

edited

Loirooriol Nov 29, 2022

webkit-commit-queue commented Nov 29, 2022

border-image-* returns the initial keyword instead of the initial value when setting border-image shorthand #6652

border-image-* returns the initial keyword instead of the initial value when setting border-image shorthand #6652

Conversation

darinadler commented Nov 18, 2022 • edited by webkit-early-warning-system

c0c6ad1

webkit-early-warning-system commented Nov 18, 2022 • edited

webkit-early-warning-system commented Nov 18, 2022 • edited

webkit-early-warning-system commented Nov 18, 2022 • edited

webkit-early-warning-system commented Nov 18, 2022 • edited

Loirooriol left a comment

Choose a reason for hiding this comment

Loirooriol left a comment

Choose a reason for hiding this comment

darinadler commented Nov 19, 2022

webkit-early-warning-system commented Nov 19, 2022 • edited

webkit-early-warning-system commented Nov 19, 2022 • edited

darinadler commented Nov 19, 2022

Loirooriol commented Nov 19, 2022

weinig commented Nov 20, 2022

webkit-early-warning-system commented Nov 28, 2022 • edited

Loirooriol Nov 29, 2022

Choose a reason for hiding this comment

darinadler Nov 29, 2022 • edited

Choose a reason for hiding this comment

Loirooriol Nov 29, 2022

Choose a reason for hiding this comment

webkit-commit-queue commented Nov 29, 2022

darinadler commented Nov 18, 2022 •

edited by webkit-early-warning-system

`c0c6ad1`

webkit-early-warning-system commented Nov 18, 2022 •

edited

webkit-early-warning-system commented Nov 18, 2022 •

edited

webkit-early-warning-system commented Nov 18, 2022 •

edited

webkit-early-warning-system commented Nov 18, 2022 •

edited

webkit-early-warning-system commented Nov 19, 2022 •

edited

webkit-early-warning-system commented Nov 19, 2022 •

edited

webkit-early-warning-system commented Nov 28, 2022 •

edited

darinadler Nov 29, 2022 •

edited