Bug 187778 – Setting foreground color when editing should take color-filter into account, and report the correct foreground color for collapsed selections

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Simon Fraser (smfr)]

Setting foreground color when editing should take color-filter into account, and report the correct foreground color for collapsed selections

[Radar WebKit Bug Importer]

<rdar://problem/42350580>

[Simon Fraser (smfr)]

Created attachment 345281 [details]
Patch

[EWS Watchlist]

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

[EWS Watchlist]

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

[Simon Fraser (smfr)]

Need Sierra-specific results.

[Simon Fraser (smfr)]

Created attachment 345295 [details]
Patch

[EWS Watchlist]

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

[EWS Watchlist]

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

[Ryosuke Niwa]

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?

[EWS Watchlist]

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

[EWS Watchlist]

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

[Ryan Haddad]

This change was landed in https://trac.webkit.org/changeset/234005/webkit

[Ryan Haddad]

(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

[Simon Fraser (smfr)]

Test fix in https://trac.webkit.org/changeset/234014/webkit.

[Ryan Haddad]

Rebaselined the test for Sierra in https://trac.webkit.org/r234041