[webkit-reviews] review granted: [Bug 207789] Ensure that contenteditable carets on macCatalyst are the right color, especially in Dark Mode : [Attachment 391093] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Megan Gardner
<megan_gardner at apple.com>'s request for review:
Bug 207789: Ensure that contenteditable carets on macCatalyst are the right
color, especially in Dark Mode
https://bugs.webkit.org/show_bug.cgi?id=207789

Attachment 391093: Patch

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




--- Comment #14 from Tim Horton <thorton at apple.com> ---
Comment on attachment 391093
  --> https://bugs.webkit.org/attachment.cgi?id=391093
Patch

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

> Source/WebKit/ChangeLog:9
> +	   Because UIKit only uses label color for the caret in macCatlayst, 

macCatalyst is not spelled correctly

> Source/WebCore/editing/FrameSelection.cpp:1777
> +#endif
> +    auto* rootEditableElement = node ? node->rootEditableElement() :
nullptr;

You have two `returns` and lots of extra unreachable code on iOS; you should
have an #else instead

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:3226
> +    // On macCatalyst we need to explicitly return the color we have
calculated, rather than rely on textTraits, as on macCatalyst, UIKit ignores
text traits, and relies on AppKit's calulation, which cannot track dark mode
correctly in web content.

"calulation" is spelled incorrectly (should be "calculation")

Also, this part isn't about dark mode... they totally ignore ANY
insertionPointColor change, even from caretColor. I would just stop after
"ignores text traits" and slap a period on there and you're golden.