Bug 211178

Summary: [GTK4] Make popup menus work
Product: WebKit Reporter: Adrian Perez <aperez>
Component: WebKitGTKAssignee: Adrian Perez <aperez>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, cgarcia
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 210100    
Attachments:
Description Flags
Patch aperez: review+

Description Adrian Perez 2020-04-29 05:12:32 PDT 
Currently for popups menus windows of type GTK_WINDOW_POPUP are used,
but this is not possible with GTK4 anymore. The way to create a popup
window is using GtkPopover.

If we want to mimic GTK combo boxes, we likely would need to reimplement
parts of GtkTreePopover, which is the internal widget used as the popup
part of combo boxes. On the other hand, we may be able to put the widget
currently used inside a GtkPopover instead of inside the popup GtkWindow,
apply the same CSS classes that GtkComboBox uses, and maybe that would be
already good.

Note that ideally we want to use the same code for both GTK3 and GTK4,
as it was done recently for context menus in bug #170553 :)
Comment 1 Carlos Garcia Campos 2020-06-02 05:24:21 PDT 
Created attachment 400806 [details]
Patch
Comment 2 Adrian Perez 2020-06-02 05:42:16 PDT 
Comment on attachment 400806 [details]
Patch

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

> Source/WebKit/UIProcess/gtk/WebPopupMenuProxyGtk.cpp:335
> +    GdkRectangle windowRect = { rect.x(), rect.y(), rect.width(), rect.height() };

This exact same line is repeated below, so I would take it out of
the “#if USE(GTK4)” guard.
Comment 3 Carlos Garcia Campos 2020-06-02 05:50:49 PDT 
(In reply to Adrian Perez from comment #2)
> Comment on attachment 400806 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=400806&action=review
> 
> > Source/WebKit/UIProcess/gtk/WebPopupMenuProxyGtk.cpp:335
> > +    GdkRectangle windowRect = { rect.x(), rect.y(), rect.width(), rect.height() };
> 
> This exact same line is repeated below, so I would take it out of
> the “#if USE(GTK4)” guard.

Yes, but inside a GTK_CHECK_VERSION(3, 24, 0) if
Comment 4 Adrian Perez 2020-06-02 06:04:46 PDT 
(In reply to Carlos Garcia Campos from comment #3)
> (In reply to Adrian Perez from comment #2)
> > Comment on attachment 400806 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=400806&action=review
> > 
> > > Source/WebKit/UIProcess/gtk/WebPopupMenuProxyGtk.cpp:335
> > > +    GdkRectangle windowRect = { rect.x(), rect.y(), rect.width(), rect.height() };
> > 
> > This exact same line is repeated below, so I would take it out of
> > the “#if USE(GTK4)” guard.
> 
> Yes, but inside a GTK_CHECK_VERSION(3, 24, 0) if

You are right, all good then!
Comment 5 Carlos Garcia Campos 2020-06-02 06:05:02 PDT 
Committed r262414: <https://trac.webkit.org/changeset/262414>