Bug 138029 - Carets in GMail and iCloud compositions are the foreground text color
Summary: Carets in GMail and iCloud compositions are the foreground text color
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-10-23 15:54 PDT by Myles C. Maxfield
Modified: 2014-10-23 20:23 PDT (History)
6 users (show)

See Also:

Attachments
Patch (5.59 KB, patch)
2014-10-23 15:57 PDT, Myles C. Maxfield 		no flags Details | Formatted Diff | Diff
Patch (5.36 KB, patch)
2014-10-23 16:16 PDT, Myles C. Maxfield 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2014-10-23 15:54:09 PDT 
Carets in GMail and iCloud compositions are the foreground text color
Comment 1 Myles C. Maxfield 2014-10-23 15:57:25 PDT 
Created attachment 240374 [details]
Patch
Comment 2 Myles C. Maxfield 2014-10-23 15:58:28 PDT 
<rdar://problem/18755592>
Comment 3 Myles C. Maxfield 2014-10-23 16:03:12 PDT 
Comment on attachment 240374 [details]
Patch

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

> Source/WebCore/editing/FrameSelection.cpp:1483
> +    if (!topColor.alpha())

This needs to be topColor.hasAlpha()
Comment 4 Simon Fraser (smfr) 2014-10-23 16:06:14 PDT 
Comment on attachment 240374 [details]
Patch

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

> Source/WebCore/editing/FrameSelection.cpp:1477
> +static inline bool layeredColorsAppearSame(Color bottomColor, Color topColor)

Not a huge fan of the name (would prefer something like colorDistinctOverColor() or something),

Can this just call blend() and check if the result == bottomColor?

> Source/WebCore/editing/FrameSelection.cpp:1481
> +    if (bottomColor.red() == topColor.red() && bottomColor.green() == topColor.green() && bottomColor.blue() == topColor.blue())

Would be nice to push this into Color. (colorComponentsEqual or something).

> Source/WebCore/editing/FrameSelection.cpp:1483
> +    if (!topColor.alpha())

hasAlpha? Do you care about how much alpha?
Comment 5 Myles C. Maxfield 2014-10-23 16:16:18 PDT 
Created attachment 240375 [details]
Patch
Comment 6 WebKit Commit Bot 2014-10-23 16:49:04 PDT 
Comment on attachment 240375 [details]
Patch

Clearing flags on attachment: 240375

Committed r175152: <http://trac.webkit.org/changeset/175152>
Comment 7 WebKit Commit Bot 2014-10-23 16:49:08 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 8 Joseph Pecoraro 2014-10-23 17:33:33 PDT 
Seems iOS builds are complaining:

Source/WebCore/editing/FrameSelection.cpp:1477:20: error: unused function 'disappearsIntoBackground' [-Werror,-Wunused-function]
static inline bool disappearsIntoBackground(Color foreground, Color background)
1 error generated.
Comment 9 Darin Adler 2014-10-23 20:16:31 PDT 
Comment on attachment 240375 [details]
Patch

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

> Source/WebCore/editing/FrameSelection.cpp:1480
> +static inline bool disappearsIntoBackground(Color foreground, Color background)
> +{
> +    return background.blend(foreground) == background;
> +}

Needs to be wrapped in #if ENABLE(TEXT_CARET).

Also, is == too strict? Should we have some threshold for close colors?
Comment 10 Simon Fraser (smfr) 2014-10-23 20:23:03 PDT 
Build issue was fixed in http://trac.webkit.org/changeset/175155

I mentioned the perceptive matching issue to Myles; he didn't want to get into that rat hole.