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 692697 - Ctrl+scrollwheel no longer works for zooming
Ctrl+scrollwheel no longer works for zooming
Status: RESOLVED OBSOLETE
Product: epiphany
Classification: Core
Component: Interface
3.27.x
Other Linux
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
: 696293 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-01-28 11:22 UTC by Reinout van Schouwen
Modified: 2018-08-03 19:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Adding ctl+scroll wheel zooming (2.04 KB, patch)
2018-02-10 21:22 UTC, Jan-Michael Brummer
needs-work Details | Review

Description Reinout van Schouwen 2013-01-28 11:22:24 UTC
With the upgrade to Fedora 18 came Epiphany 3.6.1.
Epiphany always supported zooming via Ctrl+scrollwheel, but this is no longer the case.
I expect to be able to zoom in by holding Ctrl and scrolwheel up, and zoom out by Ctrl+scrollwheel down.
Comment 1 Xan Lopez 2013-03-21 16:10:23 UTC
*** Bug 696293 has been marked as a duplicate of this bug. ***
Comment 2 Reinout van Schouwen 2013-03-22 10:58:48 UTC
Which distro are you using, Vitezslav? This bug only seems to be observed on Fedora.
Comment 3 Xan Lopez 2013-03-22 12:07:07 UTC
I assume Fedora patched this somehow in 3.6.x, but this feature cannot work at all in 3.7.x because it was removed.
Comment 4 Reinout van Schouwen 2013-03-22 15:47:36 UTC
(In reply to comment #3)
> I assume Fedora patched this somehow in 3.6.x, but this feature cannot work at
> all in 3.7.x because it was removed.

Really? Why remove something that makes life _so much_ easier? With the zoom buttons also no longer being on the toolbar, quickly setting a comfortable zoom level is getting pretty hard!
Comment 5 Xan Lopez 2013-03-22 16:38:25 UTC
It was removed because with the WK2 transition it was breaking heavily pages with subframes (making them unusable). I'll more than happy to reimplement it if it can be done in a safe way.
Comment 6 Reinout van Schouwen 2013-03-22 22:37:21 UTC
I see. Should this bug depend on the WK2 tracker bug then?
Comment 7 Jan-Michael Brummer 2018-02-10 21:22:22 UTC
Created attachment 368223 [details] [review]
Adding ctl+scroll wheel zooming

Please review. Note: this patch can be refactored as the zooming code is duplicated and can be merged into one function. Into embed?
Comment 8 Michael Catanzaro 2018-02-11 20:19:08 UTC
Review of attachment 368223 [details] [review]:

Where exactly is this code duplicated?

::: embed/ephy-web-view.c
@@ +509,3 @@
+
+  /* Let WebKitWebView handle this. */
+  return GTK_WIDGET_CLASS (ephy_web_view_parent_class)->scroll_event (widget, event);

Problem is, some websites are going to expect to be able to handle ctrl+scroll themselves. I bet this breaks e.g. maps.google.com, can you check Ctrl+scroll there before and after this patch? I notice GTK MiniBrowser tries to implement this feature, but in doing so breaks Google Maps.

I think we need to first pass the event to the web view, check if the web view handled it, and then handle it ourselves only if the web view did not. But I'm not sure whether that's actually possible to do or not. Event handling in GTK+ is synchronous, but in WebKit it is asynchronous. I fear WebKit might always handle the event, to prevent the GTK+ event handler to hang the UI process while WebKit passes the event around via IPC to figure out whether to handle it or not. So first thing to check is if that's true or not. Does GTK_WIDGET_CLASS (ephy_web_view_parent_class)->scroll_event (widget, event) always return GDK_EVENT_STOP, or does it choose between returning GDK_EVENT_PROPAGATE or GDK_EVENT_STOP based on whether the web content tried to handle the event? If it's the later, then we can handle it ourselves in the case where it returns PROPAGATE. But I fear it will likely be the former. If so, I'm actually not certain what we should do here. I don't want to say "no! we cannot implement this basic feature that all other browsers have!" but this might be quite hard to get right.
Comment 9 Jan-Michael Brummer 2018-02-11 20:25:42 UTC
I've switched the order, unfortuantely webkit consumes the event and always returns TRUE in my tests.
Comment 10 Michael Catanzaro 2018-02-11 20:34:19 UTC
Timm, do you know much about GTK+ event handling?

The result is as I feared. I doubt this will be possible to fix this as long as we use GTK+ 3. We'd need to fundamentally change GTK+ event handling to allow asynchronous decisions. I know event handling has been reworked in GTK+ 4, but I don't know anything about how so; if the new event handling model still expects an immediate, synchronous decision, then we're not any better off. If it allows for asynchronous decisions, then we have the possibility to fix this in WebKit.

The way to fix this bug would be to:

 * Change GTK+ to not use signal handlers for event propagation, or to at least allow an opt-out so that events can be handled asynchronously. (Is this already the case in GTK+ 4?)
 * Change event handling in WebKitWebViewBase accordingly.
 * Port WebKitGTK+ to GTK+ 4, or whatever new version of GTK+ that allows for asynchronous event handling. (This will be painful.)

Then, with all that done, we could handle ctrl+scroll here in Epiphany. But really, WebKitWebViewBase should probably handle the zooming itself instead; Ctrl+scroll is pretty basic and IMO could be handled by WebKit rather than at the browser level.
Comment 11 Jan-Michael Brummer 2018-02-11 20:38:45 UTC
At a first look it should be changed in webkit, as the scroll_event cb returns always GDK_EVENT_STOP without looking at the result of webkitWebViewBaseHandleWheelEvent ()
Comment 12 Timm Bäder 2018-02-11 20:41:31 UTC
Can you elaborate on what you mean by asynchronous event handling or asynchronous decisions? GTK4 uses event handlers (and gestures) instead of the event signals so it's different, but I'm not sure if those would solve your problem. If not, it might be better to ask garnacho as he's the mastermind behind the input stuff.
Comment 13 Michael Catanzaro 2018-02-12 15:54:34 UTC
(In reply to Timm Bäder from comment #12)
> Can you elaborate on what you mean by asynchronous event handling or
> asynchronous decisions?

To decide whether to handle the event or not, WebKit has to perform IPC. If you wait for WebKit to decide whether it wants to handle the event, the UI process will hang for an unknown amount of time. That's unacceptable, so WebKit instead always pretends to handle the event. What we need is a way to tell GTK+ that we will decide asynchronously, at some point in the future, whether or not to handle the event. Event propagation should wait until that time.

(This is, of course, a longstanding problem.)

(In reply to Jan-Michael Brummer from comment #11)
> At a first look it should be changed in webkit, as the scroll_event cb
> returns always GDK_EVENT_STOP without looking at the result of
> webkitWebViewBaseHandleWheelEvent ()

Look at how webkitWebViewBaseHandleWheelEvent is implemented. It passes the wheel event on to WebPageProxy. WebPageProxy sends it to the web process, for the web process to decide whether to handle it. It's simply impossible to get an immediate decision. So returning GDK_EVENT_STOP is the best that can be done. If it were to return GDK_EVENT_PROPAGATE then we could wind up with the event being processed twice, once by the web content and once by the application.
Comment 14 Jan-Michael Brummer 2018-02-12 17:30:35 UTC
Is it possible to move the complete handling of ctrl + scrollwheel to webkit?
Comment 15 Michael Catanzaro 2018-02-12 21:54:53 UTC
(In reply to Jan-Michael Brummer from comment #14)
> Is it possible to move the complete handling of ctrl + scrollwheel to webkit?

Should be, yes. Actually, that would suffice in this case. I'd recommend pursuing that route, because we really don't need this functionality at the Epiphany level at all.

No doubt this GTK+ event handling limitation will bite us again in the future, though....
Comment 16 GNOME Infrastructure Team 2018-08-03 19:48:48 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/epiphany/issues/184.