Bug 47631 – Draw the find overlay holes

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Anders Carlsson]

Draw the find overlay holes

[Anders Carlsson]

Created attachment 70677 [details]
Patch

[John Sullivan]

Comment on attachment 70677 [details]
Patch

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

See my comment above about where to put the constant definitions. I don't have a horse in that race, though.

> WebKit2/ChangeLog:10
> +        Move the color component constants into overlayBackgroundColor.

I'm not sure what the style guidelines say about this. I'm used to putting all the static consts at the top of the file. It seems odd to put the color ones inside overlayBackgroundColor, but not put the shadow ones inside drawRect.

Comment on attachment 70677 [details]
Patch

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

> WebKit2/WebProcess/WebPage/FindPageOverlay.cpp:92
> +static const float shadowOffsetX = 0.0;
> +static const float shadowOffsetY = 1.0;
> +static const float shadowBlurRadius = 2.0;
> +static const float whiteFrameThickness = 1.0;
> +

Please remove the “.0”s.

> WebKit2/WebProcess/WebPage/FindPageOverlay.cpp:115
>      graphicsContext.fillRect(paintRect, overlayBackgroundColor(), sRGBColorSpace);

I wonder if this shouldn’t have been in device space.

> WebKit2/WebProcess/WebPage/FindPageOverlay.cpp:125
> +        whiteFrameRect.inflate(1);

Did you mean to use whiteFrameThickness?

[Anders Carlsson]

Committed r69711: <http://trac.webkit.org/changeset/69711>