[webkit-reviews] review granted: [Bug 187778] Setting foreground color when editing should take color-filter into account, and report the correct foreground color for collapsed selections : [Attachment 345295] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 187778: Setting foreground color when editing should take color-filter into
account, and report the correct foreground color for collapsed selections
https://bugs.webkit.org/show_bug.cgi?id=187778

Attachment 345295: Patch

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




--- Comment #9 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 345295
  --> https://bugs.webkit.org/attachment.cgi?id=345295
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-col
or-filter.html:26
> +}

Show an error message here in else case?