Bug 186566 - Semantic colors should not be transformed by color-filter
Summary: Semantic colors should not be transformed by color-filter
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-06-12 10:08 PDT by Antti Koivisto
Modified: 2018-06-15 13:52 PDT (History)
8 users (show)

See Also:


Attachments
patch (8.63 KB, patch)
2018-06-12 10:34 PDT, Antti Koivisto
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-sierra (582.75 KB, application/zip)
2018-06-12 11:27 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews106 for mac-sierra-wk2 (869.39 KB, application/zip)
2018-06-12 12:26 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews125 for ios-simulator-wk2 (2.23 MB, application/zip)
2018-06-12 12:31 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews113 for mac-sierra (1.35 MB, application/zip)
2018-06-12 12:58 PDT, EWS Watchlist
no flags Details
patch (13.12 KB, patch)
2018-06-13 05:20 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (13.77 KB, patch)
2018-06-13 05:57 PDT, Antti Koivisto
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews124 for ios-simulator-wk2 (2.38 MB, application/zip)
2018-06-13 07:53 PDT, EWS Watchlist
no flags Details
patch (13.76 KB, patch)
2018-06-13 07:57 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (15.30 KB, patch)
2018-06-15 07:31 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.