Bug 187778

Summary: Setting foreground color when editing should take color-filter into account, and report the correct foreground color for collapsed selections
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: New BugsAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, rniwa, ryanhaddad, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews101 for mac-sierra
none
Patch
rniwa: review+, ews-watchlist: commit-queue-
Archive of layout-test-results from ews101 for mac-sierra
none
Archive of layout-test-results from ews205 for win-future none

Description Simon Fraser (smfr) 2018-07-18 13:45:34 PDT
Setting foreground color when editing should take color-filter into account, and report the correct foreground color for collapsed selections
Comment 1 Radar WebKit Bug Importer 2018-07-18 14:06:03 PDT
<rdar://problem/42350580>
Comment 2 Simon Fraser (smfr) 2018-07-18 14:07:31 PDT
Created attachment 345281 [details]
Patch
Comment 3 EWS Watchlist 2018-07-18 15:19:43 PDT
Comment on attachment 345281 [details]
Patch

Attachment 345281 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8579271

New failing tests:
editing/mac/attributed-string/attributed-string-for-typing-with-color-filter.html
editing/mac/attributed-string/attrib-string-colors-with-color-filter.html
editing/mac/attributed-string/attrib-string-range-with-color-filter.html
Comment 4 EWS Watchlist 2018-07-18 15:19:45 PDT
Created attachment 345293 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 5 Simon Fraser (smfr) 2018-07-18 15:20:27 PDT
Need Sierra-specific results.
Comment 6 Simon Fraser (smfr) 2018-07-18 15:35:04 PDT
Created attachment 345295 [details]
Patch
Comment 7 EWS Watchlist 2018-07-18 16:34:39 PDT
Comment on attachment 345295 [details]
Patch

Attachment 345295 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8580467

New failing tests:
editing/mac/attributed-string/attributed-string-for-typing-with-color-filter.html
Comment 8 EWS Watchlist 2018-07-18 16:34:41 PDT
Created attachment 345303 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 9 Ryosuke Niwa 2018-07-18 21:38:06 PDT
Comment on attachment 345295 [details]
Patch

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

> Source/WebCore/editing/cocoa/EditorCocoa.mm:104
>      if (foregroundColor.isValid() && !Color::isBlackColor(foregroundColor))

What we really want to check here is whether foregroundColor matches the default text color or not.

> LayoutTests/editing/mac/attributed-string/attrib-string-range-with-color-filter.html:14
> +        document.getElementById("console").appendChild(document.createTextNode(message + "\n"));

Why not just document.getElementById("console").append(message + '\n')?

> LayoutTests/editing/mac/attributed-string/attributed-string-for-typing-with-color-filter.html:26
> +}

Show an error message here in else case?
Comment 10 EWS Watchlist 2018-07-18 22:56:10 PDT
Comment on attachment 345295 [details]
Patch

Attachment 345295 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8583440

New failing tests:
editing/style/exec-command-foreColor-with-color-filter.html
Comment 11 EWS Watchlist 2018-07-18 22:56:22 PDT
Created attachment 345331 [details]
Archive of layout-test-results from ews205 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews205  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 12 Ryan Haddad 2018-07-19 16:07:51 PDT
This change was landed in https://trac.webkit.org/changeset/234005/webkit
Comment 13 Ryan Haddad 2018-07-19 16:08:10 PDT
(In reply to Build Bot from comment #7)
> Comment on attachment 345295 [details]
> Patch
> 
> Attachment 345295 [details] did not pass mac-ews (mac):
> Output: https://webkit-queues.webkit.org/results/8580467
> 
> New failing tests:
> editing/mac/attributed-string/attributed-string-for-typing-with-color-filter.
> html
This is failing on the bots now that the change has landed:
https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK1%20(Tests)/r234007%20(6810)/results.html
Comment 14 Simon Fraser (smfr) 2018-07-19 16:21:15 PDT
Test fix in https://trac.webkit.org/changeset/234014/webkit.
Comment 15 Ryan Haddad 2018-07-20 09:27:55 PDT
Rebaselined the test for Sierra in https://trac.webkit.org/r234041