[webkit-reviews] review denied: [Bug 59183] Overlay scroller hard to see on pages with dark background : [Attachment 90683] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Apr 22 09:44:21 PDT 2011
Sam Weinig <sam at webkit.org> has denied Jon Lee <jonlee at apple.com>'s request for
review:
Bug 59183: Overlay scroller hard to see on pages with dark background
https://bugs.webkit.org/show_bug.cgi?id=59183
Attachment 90683: Patch
https://bugs.webkit.org/attachment.cgi?id=90683&action=review
------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=90683&action=review
> Source/WebCore/platform/mac/ScrollbarThemeMac.mm:214
> + if (!scrollbar->parent()->isFrameView())
> + return ScrollbarOverlayStyleDefault;
> +
> + FrameView* frameView = static_cast<FrameView*>(scrollbar->parent());
> + Document* document = frameView->frame()->document();
> + Element* rootElementToUse = document->body();
> + if (!rootElementToUse)
> + rootElementToUse = document->documentElement();
> + if (!rootElementToUse)
> + return ScrollbarOverlayStyleDefault;
> +
> + RenderObject* renderer = rootElementToUse->renderer();
> + if (!renderer)
> + return ScrollbarOverlayStyleDefault;
> + Color color =
renderer->style()->visitedDependentColor(CSSPropertyBackgroundColor);
> + if (!color.isValid())
> + return ScrollbarOverlayStyleDefault;
> +
> + // reduce the background color from RGB to a lightness value
> + // and determine which scrollbar style to use based on a magic
> + // lightness heuristic
> + double h, s, l;
> + color.getHSL(h, s, l);
> + if (l < .5)
> + return ScrollbarOverlayStyleLight;
This is a layering violation, since classes in WebCore/platform are not
supposed to know about rendering, the DOM, or the frame tree. Instead, I think
you should add a virtual method to ScrollableArea which FrameView can implement
to return the background color. This will also make this possible to implement
for overflow areas in the future.
More information about the webkit-reviews
mailing list