[Webkit-unassigned] [Bug 44862] Hard to find a caret in an editable element with black background color

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=44862





--- Comment #10 from Ryosuke Niwa <rniwa at webkit.org>  2010-09-02 04:01:54 PST ---
(From update of attachment 66349)
> +        This patch makes the caret color to be changed to black or white depends on the lightness of background. 
> +
> +        No new tests needed.

I think we need tests. It's just that we cannot test it.

> +        Element* parentElement = element;
> +        bool isTransparentPage = false;
> +        while (!parentElement->renderer()->style()->hasBackground()) {
> +            if (parentElement->hasTagName(bodyTag)) {
> +                isTransparentPage = true;
> +                break;
> +            }
> +            parentElement = parentElement->parentElement();
> +        }

Some element may not have renderer so you should check that.  Also, did you consider the case where the document is not HTML?  We may never hit body element in that case.

> +        if (!isTransparentPage) {
> +            backgroundColor = parentElement->renderer()->style()->visitedDependentColor(CSSPropertyBackgroundColor);
> +            if (backgroundColor.lightness() < 0.5)
> +                caretColor = Color::white;
> +        } else
> +            caretColor = Color::black;

LGTM as long as you add the null check for renderer in the loop.

> +double Color::lightness() const
> +{
> +    double r = static_cast<double>(red()) / 255.0;
> +    double g = static_cast<double>(green()) / 255.0;
> +    double b = static_cast<double>(blue()) / 255.0;
> +    double max = std::max(std::max(r, g), b);
> +    double min = std::min(std::min(r, g), b);
> +    return 0.5 * (max + min);
> +}

This code looks rather arbitrary to me.  Does this match other browser's behavior, namely that of MSIE?
Also, I'm not sure if it's a good idea to add this to Color since Color is used everywhere in WebKit
and we want to keep it as small as possible.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.