Make '-webkit-background-size' a legacy shorthand#3372
Make '-webkit-background-size' a legacy shorthand#3372webkit-early-warning-system merged 1 commit intoWebKit:mainfrom
Conversation
There was a problem hiding this comment.
That's not good, there is no round-trip:
element.style.backgroundSize = "100px"; // behaves as "100px auto"
element.style.webkitBackgroundSize; // "100px", oops!
element.style.webkitBackgroundSize += ""; // changes behavior to "100px 100px"element.style.webkitBackgroundSize = "100px"; // behaves as "100px 100px"
element.style.backgroundSize; // "100px", oops!
element.style.backgroundSize += ""; // changes behavior to "100px auto"And I guess this was already wrong, but please fix gCS while you are at it:
element.style.backgroundSize = "100px";
getComputedStyle(element).webkitBackgroundSize; // "100px", oops!This is OK:
element.style.webkitBackgroundSize = "100px";
getComputedStyle(element).backgroundSize; // "100px 100px"See whatwg/compat#28, either serialize background-size and -webkit-background-size differently, or always use 2 values. People there seemed to prefer 2 values.
There was a problem hiding this comment.
Yes, and make the unprefixed property parse like -webkit-background-size if it's web-compatible. whatwg/compat#28 (comment)
There was a problem hiding this comment.
Changing the parsing is out-of-scope for this change, and would carry more web compat risk.
I have changed serialization to always use two values.
a6fa5c8 to
1ddd2b7
Compare
|
EWS run on previous version of this PR (hash 1ddd2b7) Details |
|
EWS run on previous version of this PR (hash a6fa5c8) Details |
1ddd2b7 to
50a2514
Compare
|
EWS run on previous version of this PR (hash 50a2514) Details |
Loirooriol
left a comment
There was a problem hiding this comment.
Informal LGTM.
It would be good to fix getComputedStyle while you are at it, but it's already broken so not blocking on that.
I think Aditya's patch makes sure to always serialize to 2 values, so that would also get fixed, I would think. We could add a test for it. |
|
No, if (fillSize.size.height.isAuto())
return ComputedStyleExtractor::zoomAdjustedPixelValueForLength(fillSize.size.width, style); |
Good catch! Seems easily fixable, we could pass in a |
50a2514 to
a56e956
Compare
|
EWS run on previous version of this PR (hash a56e956) Details |
a56e956 to
e226b87
Compare
|
EWS run on previous version of this PR (hash e226b87) Details
|
|
Following the change to gCS, the following WPTs are failing: They expect a single value for @nt1m @zcorpan Any suggestions on how to proceed with gCS / these WPT? |
|
@pxlcoder You can either change the expected result, or use |
e226b87 to
99a4aaf
Compare
|
EWS run on current version of this PR (hash 99a4aaf) Details
|
|
EWS run on previous version of this PR (hash 99a4aaf) Details |
nt1m
left a comment
There was a problem hiding this comment.
Can you add a GTK specific baseline for:
LayoutTests/imported/w3c/web-platform-tests/css/cssom/getComputedStyle-pseudo.html ?
This is actually not a regression of your patch but of a3ee3aa , but would be nice to fix anyway here since your patch touches the baseline.
There was a problem hiding this comment.
Can you make the propertyID the first argument to be consistent with createPositionListForLayer and createSingleAxisPositionValueForLayer?
99a4aaf to
4682b66
Compare
|
EWS run on previous version of this PR (hash 4682b66) Details
|
|
iOS needs a similar rebaseline for LayoutTests/imported/w3c/web-platform-tests/css/cssom/getComputedStyle-pseudo.html |
Not sure what's going on here – the patch contains a baseline in |
|
(turns out there's already a baseline in |
4682b66 to
5325011
Compare
|
EWS run on current version of this PR (hash 5325011) Details
|
https://bugs.webkit.org/show_bug.cgi?id=243562 rdar://98151497 Reviewed by Tim Nguyen. `background-size` and `-webkit-background-size` are a pair of properties that share a computed style. `-webkit-background-size` differs from `background-size` in the interpretation of a single value: `-webkit-background-size: l;` is equivalent to `background-size: l l;`, whereas `background-size: l;` is equivalent to `background-size: l auto;`. Property pairs that share a computed style but differ in syntax should be implemented as legacy shorthands. Additionally, always serialize `background-size` and `-webkit-background-size` as two values, so that values can be roundtripped between the prefixed and unprefixed variant. Spec: https://www.w3.org/TR/css-cascade-5/#aliasing * LayoutTests/TestExpectations: Remove expectations for a forms test that was unexpectedly setting `-webkit-background-size`, as it was not a shorthand. * LayoutTests/fast/backgrounds/background-shorthand-with-backgroundSize-style-expected.txt: * LayoutTests/fast/backgrounds/background-shorthand-with-backgroundSize-style.html: * LayoutTests/fast/backgrounds/size/parsing-background-size-values-expected.txt: * LayoutTests/fast/backgrounds/size/parsing-inherit-expected.txt: * LayoutTests/fast/backgrounds/size/resources/parsing-background-size-values.js: * LayoutTests/fast/backgrounds/size/resources/parsing-inherit.js: * LayoutTests/fast/css/getComputedStyle/computed-style-expected.txt: * LayoutTests/fast/css/getComputedStyle/computed-style-without-renderer-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html: Update test to use two-value syntax. * LayoutTests/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-backgrounds/background-332-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-backgrounds/background-size-001.html: * LayoutTests/imported/w3c/web-platform-tests/css/css-backgrounds/parsing/background-size-computed.html: * LayoutTests/imported/w3c/web-platform-tests/css/cssom/getComputedStyle-detached-subtree-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/cssom/getComputedStyle-pseudo-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/quirks/unitless-length/excluded-properties-002-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/quirks/unitless-length/excluded-properties-002.html: Include an additional check for `1234px auto`, to avoid a false positive, as `background-size` is serialized to two values. * LayoutTests/platform/gtk/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt: * LayoutTests/platform/gtk/imported/w3c/web-platform-tests/css/cssom/getComputedStyle-detached-subtree-expected.txt: * LayoutTests/platform/gtk/imported/w3c/web-platform-tests/css/cssom/getComputedStyle-pseudo-expected.txt: * LayoutTests/platform/ios-wk2/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt: * LayoutTests/platform/ios-wk2/imported/w3c/web-platform-tests/css/cssom/getComputedStyle-pseudo-expected.txt: * LayoutTests/platform/ios/imported/w3c/web-platform-tests/css/cssom/getComputedStyle-detached-subtree-expected.txt: * LayoutTests/platform/ipad/imported/w3c/web-platform-tests/css/cssom/getComputedStyle-detached-subtree-expected.txt: * LayoutTests/platform/ipad/imported/w3c/web-platform-tests/css/cssom/getComputedStyle-pseudo-expected.txt: * LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt: * LayoutTests/platform/wpe/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt: * LayoutTests/platform/wpe/imported/w3c/web-platform-tests/css/cssom/getComputedStyle-detached-subtree-expected.txt: * LayoutTests/platform/wpe/imported/w3c/web-platform-tests/css/cssom/getComputedStyle-pseudo-expected.txt: * LayoutTests/svg/css/getComputedStyle-basic-expected.txt: * Source/WebCore/css/CSSProperties.json: * Source/WebCore/css/ComputedStyleExtractor.cpp: (WebCore::fillSizeToCSSValue): Return two values from `getComputedStyle` unless the value is "cover", "contain", or "auto". (WebCore::ComputedStyleExtractor::valueForPropertyInStyle): * Source/WebCore/css/StyleProperties.cpp: (WebCore::StyleProperties::getPropertyValue const): * Source/WebCore/css/parser/CSSPropertyParser.cpp: (WebCore::consumeBackgroundSize): Always serialize `background-size` and `-webkit-background-size` as two values. (WebCore::CSSPropertyParser::parseSingleValue): (WebCore::CSSPropertyParser::parseShorthand): Canonical link: https://commits.webkit.org/254925@main
|
Committed 254925@main (856140e): https://commits.webkit.org/254925@main Reviewed commits have been landed. Closing PR #3372 and removing active labels. |
5325011 to
856140e
Compare
🧪 style
856140e
5325011