Bug 201098 – Unprefix -webkit-print-color-adjust CSS property

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Myles C. Maxfield]

https://www.w3.org/TR/css-color-adjust-1/#perf it's now named "color-adjust".

[Myles C. Maxfield]

Created attachment 377220 [details]
WIP

[Philip Jägenstedt]

Are there any existing tests for this? I had a look in https://wpt.fyi/results/css/css-color-adjust?label=master&label=experimental&aligned but can't find any, those tests are for https://drafts.csswg.org/css-color-adjust-1/#preferred

[Tim Nguyen (:ntim)]

https://github.com/w3c/csswg-drafts/issues/3880#issuecomment-844285686

[Tim Nguyen (:ntim)]

https://drafts.csswg.org/css-color-adjust-1/#print-color-adjust

[Tim Nguyen (:ntim)]

Filed a bug on the Gecko side to rename color-adjust to print-color-adjust (so color-adjust can eventually be removed from the spec).

[Tim Nguyen (:ntim)]

(In reply to Tim Nguyen (:ntim) from comment #5)
> Filed a bug on the Gecko side to rename color-adjust to print-color-adjust
> (so color-adjust can eventually be removed from the spec).

https://bugzilla.mozilla.org/show_bug.cgi?id=1747595

[Tim Nguyen (:ntim)]

Created attachment 448495 [details]
Patch

[Tim Nguyen (:ntim)]

WPT is: https://github.com/web-platform-tests/wpt/blob/2800adf9b7/css/css-color-adjust/print-color-adjust-parsing.html

[Darin Adler]

Comment on attachment 448495 [details]
Patch

Looks like we missed at least 6 files:

platform/ios-wk2/fast/css/getComputedStyle/computed-style-expected.txt
platform/ios-wk2/fast/css/getComputedStyle/computed-style-without-renderer-expected.txt
platform/ios-wk2/svg/css/getComputedStyle-basic-expected.txt
platform/mac/fast/css/getComputedStyle/computed-style-expected.txt
platform/mac/fast/css/getComputedStyle/computed-style-without-renderer-expected.txt
platform/mac/svg/css/getComputedStyle-basic-expected.txt

Assuming we update those too, r=me

Also a little disappointing there are no progressions in WPT from this. Does this mean WPT doesn’t have any tests of this property?

[Darin Adler]

Oh, I see your "WPT is" comment lists a test, but maybe one we haven’t imported yet?

[Tim Nguyen (:ntim)]

Created attachment 448509 [details]
Patch

[Tim Nguyen (:ntim)]

(In reply to Darin Adler from comment #10)
> Oh, I see your "WPT is" comment lists a test, but maybe one we haven’t
> imported yet?

Yeah, the 2 relevant tests (inheritance and parsing) need to be imported, I filed bug 234927 for that.

[Darin Adler]

(In reply to Tim Nguyen (:ntim) from comment #12)
> Yeah, the 2 relevant tests (inheritance and parsing) need to be imported, I
> filed bug 234927 for that.

As a general rule, in cases like this, I would suggest that we first import a test, add it to WebKit along with the expected failure, then fix the bug or add the feature and land the patch that includes a progression on that test.

Not insisting on that in this case, but I think it’s the best practice.

[Tim Nguyen (:ntim)]

(In reply to Darin Adler from comment #9)
> Comment on attachment 448495 [details]
> Patch
> 
> Looks like we missed at least 6 files:
> 
> platform/ios-wk2/fast/css/getComputedStyle/computed-style-expected.txt
> platform/ios-wk2/fast/css/getComputedStyle/computed-style-without-renderer-
> expected.txt
> platform/ios-wk2/svg/css/getComputedStyle-basic-expected.txt
> platform/mac/fast/css/getComputedStyle/computed-style-expected.txt
> platform/mac/fast/css/getComputedStyle/computed-style-without-renderer-
> expected.txt
> platform/mac/svg/css/getComputedStyle-basic-expected.txt

r287460 removed the mac files and r287403 renamed the ios-wk2 ones to ios. The remaining ones should already be handled in the patch.

(In reply to Darin Adler from comment #13)
> (In reply to Tim Nguyen (:ntim) from comment #12)
> > Yeah, the 2 relevant tests (inheritance and parsing) need to be imported, I
> > filed bug 234927 for that.
> 
> As a general rule, in cases like this, I would suggest that we first import
> a test, add it to WebKit along with the expected failure, then fix the bug
> or add the feature and land the patch that includes a progression on that
> test.
> 
> Not insisting on that in this case, but I think it’s the best practice.

I agree, I didn't do it in this case since I was planning to update my build first to tip of webkit (which requires a new XCode SDK) before running the tests.

The WPT don't add much over the current webkit LayoutTest, aside from inheritance testing, so I thought that we could land this first. I'll get to bug 234927 ASAP.

[Darin Adler]

(In reply to Tim Nguyen (:ntim) from comment #14)
> r287460 removed the mac files and r287403 renamed the ios-wk2 ones to ios.
> The remaining ones should already be handled in the patch.

I goy that impression because of the mac-wk2 failure on EWS; I guess you dealt with that and I just didn’t know the details.

[Radar WebKit Bug Importer]

<rdar://problem/87210305>

[EWS]

Committed r287712 (245797@main): <https://commits.webkit.org/245797@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 448509 [details].

[Tim Nguyen (:ntim)]

*** Bug 282929 has been marked as a duplicate of this bug. ***