[GTK] Scrollbars are drawn behind the window resize grip

• Component: WebKit2
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

We need to pass the main window resize grip size to the WebProcess.

[Carlos Garcia Campos]

Created attachment 120791 [details]
Patch

[WebKit Review Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Martin Robinson]

Comment on attachment 120791 [details]
Patch

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

This patch should probably also add support for resizing the scrollbars. Right now the information is just ignored. We also need to consider how to handle the situation where the resize grip and the scrollbar don't intersect. For instance, one such situation would be when the WebView doesn't touch the bottom of the window at all.
\\

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:75
> +static void webkitWebViewBaseNotifyResizerSizeForWindow(WebKitWebViewBase* webViewBase, GtkWindow* window)

I think you can just eliminate this completely and call toplevelWindowResizeGripVisibilityChanged(G_OBJECT(toplevel),  0, webView) below.

[Carlos Garcia Campos]

(In reply to comment #3)
> (From update of attachment 120791 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=120791&action=review
> 
> This patch should probably also add support for resizing the scrollbars. 

I don't get what you mean.

> Right now the information is just ignored. 

what information?

> We also need to consider how to handle the situation where the resize grip and the scrollbar don't intersect. 

how can that happen?

> For instance, one such situation would be when the WebView doesn't touch the bottom of the window at all.
> \\

There are no scrollbars in that case no? This patch simply sends to the WebProcess the size of the resize grip, and fixes the problem of scrollbar button being painted behind the window resize grip. 

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:75
> > +static void webkitWebViewBaseNotifyResizerSizeForWindow(WebKitWebViewBase* webViewBase, GtkWindow* window)
> 
> I think you can just eliminate this completely and call toplevelWindowResizeGripVisibilityChanged(G_OBJECT(toplevel),  0, webView) below.

I don't like it, I prefer to have two methods, because when you read that you always have to think what that 0 is.

[Martin Robinson]

(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 120791 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=120791&action=review
> > 
> > This patch should probably also add support for resizing the scrollbars. 
> 
> I don't get what you mean.
> 
> > Right now the information is just ignored. 
> 
> what information?

Ah, I didn't realize that there was existing support for handling the resize grip in the WebProcess! Nice.

> > We also need to consider how to handle the situation where the resize grip and the scrollbar don't intersect. 
> 
> how can that happen?

This can happen when the WebView widget does not intersect with the resize grip. Imagine a window that has three stacked WebViews. Only the bottom WebView intersects the resize grip. I think all you need to do is to intersect the resize grip rectangle with the WebView rectangle.

> I don't like it, I prefer to have two methods, because when you read that you always have to think what that 0 is.

This can be handled by adding a comment after the zero like 

... 0 /* parameter spec */ ..

[Carlos Garcia Campos]

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > (From update of attachment 120791 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=120791&action=review
> > > 
> > > This patch should probably also add support for resizing the scrollbars. 
> > 
> > I don't get what you mean.
> > 
> > > Right now the information is just ignored. 
> > 
> > what information?
> 
> Ah, I didn't realize that there was existing support for handling the resize grip in the WebProcess! Nice.
> 
> > > We also need to consider how to handle the situation where the resize grip and the scrollbar don't intersect. 
> > 
> > how can that happen?
> 
> This can happen when the WebView widget does not intersect with the resize grip. Imagine a window that has three stacked WebViews. Only the bottom WebView intersects the resize grip. I think all you need to do is to intersect the resize grip rectangle with the WebView rectangle.

I guess that should be handled by WebProcess or WebCore code, if it's not already done, not by this patch, as I said this just sends the resize grip size to the web process.

> > I don't like it, I prefer to have two methods, because when you read that you always have to think what that 0 is.
> 
> This can be handled by adding a comment after the zero like 
> 
> ... 0 /* parameter spec */ ..

Still, having two methods doesn't hurt and they represent better what they do, the first time visibility of the resize grip hasn't changed.

[Carlos Garcia Campos]

Created attachment 120934 [details]
Updated patch

As I said WebCore already takes care of checking whether the scrollbar overlaps with the resize grip, but it uses the FrameView rect instead of the window rect. So, we can check it from the UI process and send and empty size when the scrollbar doesn't overlaps with the resize grip.

[Martin Robinson]

Comment on attachment 120934 [details]
Updated patch

Thanks.

[Carlos Garcia Campos]

Committed r104017: <http://trac.webkit.org/changeset/104017>