Bug 68533

Summary: [GTK]Popup drop-down menu contains extra empty spaces at beginning and occupies whole screen when items in popup reaches the height of display device
Product: WebKit Reporter: Wajahat Siddiqui <mdwajahatali.siddiqui>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, mrobinson, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: Linux   
Bug Depends on: 71831    
Bug Blocks:    
Attachments:
Description Flags
screenshot of issue
none
Proposed fix
mrobinson: review-
screenshot with fix
none
proposed patch updated mrobinson: review+, mrobinson: commit-queue-

Description Wajahat Siddiqui 2011-09-21 07:40:07 PDT
Created attachment 108155 [details]
screenshot of issue

Popup drop-down menu contains extra empty spaces at beginning and occupies whole screen when items in popup reaches the height of display device


Steps to reproduce:
1) Open www.google.com in MiniBrowser or GtkLauncher and click on advance search link on top right or goto
   http://www.google.com/advanced_search?hl=en
2) select language combobox

Observation 
* empty lines are seen at the beginning once scroll down and scroll up these lines are gone
* popup window occupies the whole screen
(screenshot attached)

RCA: As push_in is set to true for callback GtkMenuPositionFunc gets called on gtk_menu_popup 
GTK+ is trying to push the window into the visible area when 
part of the menu is outside the monitor (see http://developer.gnome.org/gtk3/3.1/GtkMenu.html#GtkMenuPositionFunc) 

Setting push_in to false will fix this issue. (No idea if this impacts other use-cases ?)
Comment 1 Wajahat Siddiqui 2011-09-21 07:53:43 PDT
Created attachment 108159 [details]
Proposed fix

proposed fix
Comment 2 Martin Robinson 2011-09-21 09:52:16 PDT
How does the giant menu look after your patch?
Comment 3 Wajahat Siddiqui 2011-09-21 21:43:15 PDT
Created attachment 108275 [details]
screenshot with fix
Comment 4 Wajahat Siddiqui 2011-09-21 21:44:23 PDT
(In reply to comment #2)
> How does the giant menu look after your patch?

It opens from where it is supposed to be 
as seen in attachment (screenshot with fix)
Comment 5 Martin Robinson 2011-09-21 23:14:47 PDT
Comment on attachment 108159 [details]
Proposed fix

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

This seems like a reasonable change. With the fix below, I'll r+ this.

> Source/WebCore/ChangeLog:12
> +        No new tests. (OOPS!)
> +

Please remove this line from the ChangeLog.
Comment 6 Wajahat Siddiqui 2011-09-21 23:38:45 PDT
Created attachment 108282 [details]
proposed patch updated

updated as per comments
Comment 7 Martin Robinson 2011-09-22 08:38:04 PDT
Comment on attachment 108282 [details]
proposed patch updated

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

> Source/WebCore/ChangeLog:8
> +        [GTK]Popup drop-down menu contains extra empty spaces at beginning and 
> +        occupies whole screen when items in popup reaches the height of display device.
> +        https://bugs.webkit.org/show_bug.cgi?id=68533
> +
> +        setting push_in to false as we dont want GTK+ to handle popup menu.
> +

Sorry. I missed a few other things here. The first line should be the bug title. All stenences should start with a capital letter and end with a period. I'll fix this and land it.
Comment 8 Martin Robinson 2011-09-22 09:07:50 PDT
Committed r95720: <http://trac.webkit.org/changeset/95720>
Comment 9 Wajahat Siddiqui 2011-09-22 21:43:40 PDT
(In reply to comment #7)
> (From update of attachment 108282 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=108282&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        [GTK]Popup drop-down menu contains extra empty spaces at beginning and 
> > +        occupies whole screen when items in popup reaches the height of display device.
> > +        https://bugs.webkit.org/show_bug.cgi?id=68533
> > +
> > +        setting push_in to false as we dont want GTK+ to handle popup menu.
> > +
> 
> Sorry. I missed a few other things here. The first line should be the bug title. All stenences should start with a capital letter and end with a period. I'll fix this and land it.

oops, I will keep this in mind, anyways thanks Martin.