[Webkit-unassigned] [Bug 73235] [Chromium] Support adding/removing page overlay to WebView

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Dec 1 15:16:55 PST 2011


--- Comment #25 from James Robinson <jamesr at chromium.org>  2011-12-01 15:16:50 PST ---
(From update of attachment 117299)
View in context: https://bugs.webkit.org/attachment.cgi?id=117299&action=review

> Source/WebKit/chromium/public/WebPageOverlayClient.h:37
> +class WebPageOverlayClient {

Darin pointed out that this interface should actually be called WebPageOverlay, not Client

> Source/WebKit/chromium/public/WebPageOverlayClient.h:43
> +    // Returns dirty rect that needs repaint.
> +    virtual WebRect dirtyRect() const = 0;

I don't think this is quite accurate - what this is really returning is the rectangle that this overlay is interested in painting. There doesn't seem to be any mechanism to deal with changing the dirty rect.

I think this would be clearer if the addPageOverlay() function specified the bounds of the overlay, just as it specified the z-order, and the rule for changing it was that if you wanted to change the bounds of an overlay you needed to remove it and add it back. That would be consistent with the way you can change the z-order.

Although looking in detail we aren't using this functionality - every overlay covers the full viewport. So let's punt on this functionality for now and add it when we have a need.

> Source/WebKit/chromium/public/WebView.h:424
> +    virtual void addWebPageOverlay(WebPageOverlayClient*, int /*z-order*/) = 0;
> +    virtual void removeWebPageOverlay(WebPageOverlayClient*) = 0;

Darin let me know that we don't use the "web" prefix in the function names. So these should be called:

see the setVisibilityState() above as an example. We all apologize deeply for the inconsistencies :/

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

More information about the webkit-unassigned mailing list