Bug 186566

Summary: Semantic colors should not be transformed by color-filter
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, dino, ews-watchlist, rniwa, simon.fraser, timothy, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
ews-watchlist: commit-queue-
Archive of layout-test-results from ews103 for mac-sierra
none
Archive of layout-test-results from ews106 for mac-sierra-wk2
none
Archive of layout-test-results from ews125 for ios-simulator-wk2
none
Archive of layout-test-results from ews113 for mac-sierra
none
patch
none
patch
ews-watchlist: commit-queue-
Archive of layout-test-results from ews124 for ios-simulator-wk2
none
patch
none
patch none

Description Antti Koivisto 2018-06-12 10:08:03 PDT
Maybe?
Comment 1 Brent Fulgham 2018-06-12 10:15:07 PDT
<rdar://problem/40705739>
Comment 2 Antti Koivisto 2018-06-12 10:34:19 PDT
Created attachment 342559 [details]
patch
Comment 3 EWS Watchlist 2018-06-12 11:27:44 PDT
Comment on attachment 342559 [details]
patch

Attachment 342559 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/8149356

Number of test failures exceeded the failure limit.
Comment 4 EWS Watchlist 2018-06-12 11:27:45 PDT
Created attachment 342566 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 5 EWS Watchlist 2018-06-12 12:26:24 PDT
Comment on attachment 342559 [details]
patch

Attachment 342559 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/8150125

Number of test failures exceeded the failure limit.
Comment 6 EWS Watchlist 2018-06-12 12:26:26 PDT
Created attachment 342580 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 7 EWS Watchlist 2018-06-12 12:31:11 PDT
Comment on attachment 342559 [details]
patch

Attachment 342559 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/8149543

New failing tests:
css3/color-filters/color-filter-ignore-semantic.html
Comment 8 EWS Watchlist 2018-06-12 12:31:13 PDT
Created attachment 342582 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4
Comment 9 EWS Watchlist 2018-06-12 12:58:23 PDT
Comment on attachment 342559 [details]
patch

Attachment 342559 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/8150388

Number of test failures exceeded the failure limit.
Comment 10 EWS Watchlist 2018-06-12 12:58:25 PDT
Created attachment 342585 [details]
Archive of layout-test-results from ews113 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 11 Antti Koivisto 2018-06-13 05:20:49 PDT
Created attachment 342648 [details]
patch
Comment 12 Antti Koivisto 2018-06-13 05:57:28 PDT
Created attachment 342649 [details]
patch
Comment 13 EWS Watchlist 2018-06-13 07:53:18 PDT
Comment on attachment 342649 [details]
patch

Attachment 342649 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/8162912

New failing tests:
css3/color-filters/color-filter-ignore-semantic.html
Comment 14 EWS Watchlist 2018-06-13 07:53:19 PDT
Created attachment 342654 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4
Comment 15 Antti Koivisto 2018-06-13 07:57:09 PDT
Created attachment 342655 [details]
patch
Comment 16 Timothy Hatcher 2018-06-14 11:50:47 PDT
Comment on attachment 342655 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=342655&action=review

> Source/WebCore/rendering/RenderThemeMac.mm:600
> +                return colorFromNSColor(color, Color::Semantic);

I added two more colorFromNSColor() uses in a patch that will land before this one. Please add Color::Semantic to those calls too. See bug 186609.
Comment 17 Antti Koivisto 2018-06-15 07:31:15 PDT
Created attachment 342811 [details]
patch
Comment 18 Simon Fraser (smfr) 2018-06-15 13:09:13 PDT
Comment on attachment 342811 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=342811&action=review

> Source/WebCore/rendering/RenderTreeAsText.cpp:245
> -            if (o.parent()->style().visitedDependentColor(CSSPropertyColor) != color)
> +            if (o.parent()->style().visitedDependentColor(CSSPropertyColor).rgb() != color.rgb())

Really confusing that "rgb()" includes alpha. It looks like these comparisons are ignoring alpha.

Maybe add an explicit bool Color::rgbaEquals(const Color&) or something.
Comment 19 Antti Koivisto 2018-06-15 13:24:49 PDT
> Really confusing that "rgb()" includes alpha. It looks like these
> comparisons are ignoring alpha.

Yeah, we can just rename it to rgba()
Comment 20 WebKit Commit Bot 2018-06-15 13:52:57 PDT
Comment on attachment 342811 [details]
patch

Clearing flags on attachment: 342811

Committed r232892: <https://trac.webkit.org/changeset/232892>
Comment 21 WebKit Commit Bot 2018-06-15 13:52:59 PDT
All reviewed patches have been landed.  Closing bug.