After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 783408 - Use custom popup menu for select elements
Use custom popup menu for select elements
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: Interface
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-06-04 15:17 UTC by Carlos Garcia Campos
Modified: 2017-08-05 10:19 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Use a custom popup menu for select elements (26.63 KB, patch)
2017-06-04 15:20 UTC, Carlos Garcia Campos
committed Details | Review
Screenshot (68.05 KB, image/png)
2017-06-04 15:21 UTC, Carlos Garcia Campos
  Details

Description Carlos Garcia Campos 2017-06-04 15:17:48 UTC
Using the new WebKit API. We could use a better menu with a scrollable tree view, more consistent with other browsers.
Comment 1 Carlos Garcia Campos 2017-06-04 15:20:36 UTC
Created attachment 353146 [details] [review]
Use a custom popup menu for select elements

I wrote this to check the new WebKit API. There's actually no reason why this couldn't be the default implementation in WebKit, but I think we can try it out for a while in ephy and then move it to WebKit in the next release cycle.
Comment 2 Carlos Garcia Campos 2017-06-04 15:21:48 UTC
Created attachment 353148 [details]
Screenshot
Comment 3 Michael Catanzaro 2017-06-14 16:24:19 UTC
Sorry for the delay. Reviewing this is still on my TODO list. Feel free to push unreviewed if you're impatient, or wait, as you prefer.
Comment 4 Michael Catanzaro 2017-06-26 04:35:21 UTC
(In reply to Michael Catanzaro from comment #3)
> Sorry for the delay. Reviewing this is still on my TODO list.
Comment 5 Michael Catanzaro 2017-07-30 23:46:16 UTC
Review of attachment 353146 [details] [review]:

I forgot about this again, sorry. Tried the patch and it looks fine, though I'm not really sure it's much better than what we had before. I only skimmed the code.

::: embed/ephy-option-menu.c
@@ +317,3 @@
+
+static gboolean
+ephy_option_menu_button_press_event (GtkWidget *widget, GdkEventButton *event)

Two lines

::: embed/ephy-web-view.c
@@ +1014,3 @@
 
+static gboolean
+show_option_menu_cb (EphyWebView *web_view, WebKitOptionMenu *menu, GdkEvent* event, GdkRectangle *rect)

It's not WebKit, declare them on three lines!

@@ +1016,3 @@
+show_option_menu_cb (EphyWebView *web_view, WebKitOptionMenu *menu, GdkEvent* event, GdkRectangle *rect)
+{
+  g_assert(!web_view->option_menu);

Missing space
Comment 6 Adrian Perez 2017-08-03 09:29:14 UTC
Would it be too much to already move this implementation for the popups
into WebKitGTK+? I have been using a patched Epiphany for almost a month
now without issue, and I find this implementation more suitable for web
browsing than the default one provided by WebKitGTK+.
Comment 7 Carlos Garcia Campos 2017-08-03 10:20:22 UTC
(In reply to Adrian Perez from comment #6)
> Would it be too much to already move this implementation for the popups
> into WebKitGTK+? I have been using a patched Epiphany for almost a month
> now without issue, and I find this implementation more suitable for web
> browsing than the default one provided by WebKitGTK+.

It's too late in the release cycle and only two users have tested this patch (you and me), so better land this patch in ephy and move the impl to webkit right after branching.
Comment 8 Adrian Perez 2017-08-03 10:34:37 UTC
(In reply to Carlos Garcia Campos from comment #7)
> (In reply to Adrian Perez from comment #6)
> > Would it be too much to already move this implementation for the popups
> > into WebKitGTK+? I have been using a patched Epiphany for almost a month
> > now without issue, and I find this implementation more suitable for web
> > browsing than the default one provided by WebKitGTK+.
> 
> It's too late in the release cycle and only two users have tested this patch
> (you and me), so better land this patch in ephy and move the impl to webkit
> right after branching.

Fair enough! Let's have it in Epiphany, and move it over later on.