Closed Bug 996678 Opened 11 years ago Closed 9 years ago

[gtk3] Unexpected GDK_SCROLL_SMOOTH events when scroll button events are propagated from plugins

Categories

(Core :: Widget: Gtk, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 958868

People

(Reporter: pochu27, Unassigned)

References

Details

Attachments

(2 files)

When libxul is built against gtk3, we can receive GDK_SCROLL_SMOOTH events in addition to GDK_SCROLL_{UP,DOWN,LEFT,RIGHT}, but widget/gtk/nsWindow.cpp:nsWindow::OnScrollEvent() doesn't handle them. This isn't a problem when scrolling on the firefox window, but when scrolling inside a plugin window (e.g. a flash video) plugin-container re-sends the event to the firefox X window (see dom/plugins/ipc/PluginModuleChild.cpp:gtk_plug_scroll_event()), and for some reason we only get a _SMOOTH event but no _UP/_DOWN events. There's something odd here because we're not adding GDK_SMOOTH_SCROLL_MASK to the event mask, yet we're receiving _SMOOTH events.
Blocks: 624422
Blocks: gtk3
No longer blocks: 624422
This patch works for me. It's a hacky workaround for GTK delivering both synthetic and regular scroll events without providing a clean way to distinguish them.
Comment on attachment 8408918 [details] [diff] [review] hacky-smooth-scroll-support.diff Karl, can you check this one please?
Attachment #8408918 - Flags: feedback?(karlt)
Comment on attachment 8408918 [details] [diff] [review] hacky-smooth-scroll-support.diff IIUC this is more a patch for bug 958868 than this bug. I'll retitle this bug as the current title is a bit misleading. >- NS_ASSERTION(wheelEvent.deltaX || wheelEvent.deltaY, >- "deltaX or deltaY must be non-zero"); Would it make sense to keep this assertion? >+ if (((deltaX == 1.0 || deltaX == -1.0) && deltaY == 0.0) || >+ ((deltaY == 1.0 || deltaY == -1.0) && deltaX == 0.0)) >+ { >+ // HACK: if this event looks like it's being synthesized >+ // from a non-pixel-scroll event, then process the next >+ // regular scroll event normally. Is this necessary? What are the consequences of proceeding with the smooth scroll. >+ wheelEvent.scrollType = WidgetWheelEvent::SCROLL_SYNCHRONOUSLY; Is there a reason to force a synchronous scroll? >+ wheelEvent.deltaX = deltaX; >+ wheelEvent.deltaY = deltaY; What are the units of the scroll deltas? Is nsIDOMWheelEvent::DOM_DELTA_LINE appropriate for smooth scrolling? Should isPixelOnlyDevice be set? >+ /* Work around GNOME bug#726878 by discarding regular scroll >+ * events synthesized from pixel scroll events. */ It would be helpful to know what is causing the problem here, so we can know what is the correct fix.
Attachment #8408918 - Flags: feedback?(karlt) → feedback+
Summary: [gtk3] GDK_SCROLL_SMOOTH events not handled → [gtk3] Unexpected GDK_SCROLL_SMOOTH events when scroll button events are propagated from plugins
(In reply to Karl Tomlinson (needinfo?:karlt) from comment #3) > >+ wheelEvent.deltaX = deltaX; > >+ wheelEvent.deltaY = deltaY; > > What are the units of the scroll deltas? > Is nsIDOMWheelEvent::DOM_DELTA_LINE appropriate for smooth scrolling? > Should isPixelOnlyDevice be set? AFAIK The delta is a fraction of regular scroll size. So 1(-1) means a normal scroll size, 2(-2) means double scroll size and if the system supports the smooth events you can get a value smaller than 1.
More details about smooth vs. old scroll events - http://thread.gmane.org/gmane.comp.gnome.desktop/46513
Attached patch smooth scroll patch (deleted) — Splinter Review
Karl, what about this one? According to gnome folks (http://thread.gmane.org/gmane.comp.gnome.desktop/46513) all Gtk3 apps should use smooth scroll only. The old one is there just for a compatibility reasons. IMHO nsIDOMWheelEvent::DOM_DELTA_LINE is ok here as far as wheelEvent.deltaX/wheelEvent.deltaY can be smaller than 1. This patch discards od UP/DOWN scroll so we don't have to care about them. I tested the patch on Fedora 20 (gtk3-3.10.8) and works as expected. If we process both events (UP/DOWN and SMOOTH) the scrolling is two times faster, the UP/DOWN carry the same values (at least on my box).
Attachment #8423977 - Flags: review?(karlt)
(In reply to Martin Stránský from comment #6) > According to gnome folks > (http://thread.gmane.org/gmane.comp.gnome.desktop/46513) all Gtk3 apps > should use smooth scroll only. The old one is there just for a compatibility > reasons. I couldn't find anything saying that apps should stop using UP/DOWN. Are you able to point me at the particular post you saw this, please, and quote the relevant sentence?
http://thread.gmane.org/gmane.comp.gnome.desktop/46513 [...] "* if you are on xserver 1.12 with xinput 2.2 your application stop receiving GDK_SCROLL_DOWN and GDK_SCROLL_UP events, but receive GDK_SCROLL_SMOOTH with an increment instead that change seems to create quite some issue, it breaks for example mouse wheel scrolling over sound sliders in the control center, or scrolling in nautilus compact view Grepping around in my work tree I see there are quite a lot of GNOME components using GDK_SCROLL_UP,DOWN, I guess those will stop working as they should." [...] But I can ask gnome team for further info.
(In reply to Martin Stránský from comment #8) http://article.gmane.org/gmane.comp.gnome.desktop/46513 was a post from someone affected by the changes, and I think the part quoted on comment 8 was an observation rather than a description of how things were meant to work. That may be a bug report and http://article.gmane.org/gmane.comp.gnome.desktop/46529 seems to confirm that as a bug, where Matthias Clasen wrote: On Fri, Mar 16, 2012 at 8:50 AM, Patryk Zawadzki <patrys <at> pld-linux.org> wrote: > W dniu 16 marca 2012 13:33 użytkownik Sebastien Bacher > <seb128 <at> ubuntu.com> napisał: >> Le 16/03/2012 12:56, Bastien Nocera a écrit : >>> So gtk_widget_add_events (widget, GDK_SCROLL_MASK); for every widget for >>> which we want the old behaviour back, correct? Sounds doable by 3.4. >> Well "old behaviour back", you will still not get GDK_SCROLL_UP,DOWN events >> so any code checking for those will need updating. > > Shouldn't that be the case unless you explicitly ask for GDK_SMOOTH_SCROLL_MASK? Yes, that should be the case It's not clear (from that thread) whether the bug is in GTK or Nautilus. It's been hard to work out what is an appropriate solution without documentation on how these events interact, and the bugs mean we cannot determine the intended behavior by observation, but I'm pretty sure that GTK will not give us smooth scroll events if the mouse driver does not send them, so we still need to handle the discrete (button) scroll events. See also discussion in bug 958868.
Comment on attachment 8423977 [details] [diff] [review] smooth scroll patch Still need to handle button scroll events and lineOrPageDeltaX/Y is not right here. See bug 958868.
Attachment #8423977 - Flags: review?(karlt) → review-
Ahh, okay, we can close it as a dupe of Bug 958868 then.
We can dupe this to Bug 958868 if the fix there works around this bug, but wait until something lands. I suspect this bug is just the GTK bug https://bugzilla.gnome.org/show_bug.cgi?id=726878 which would mean this report is technically invalid.
Blocks: 1034064
No longer blocks: 1034064
I've been using the GTK3 version of firefox for a long time now, and I've always had a very annoying bug: I just can't scroll at all in the window, I have to alt-tab for scrolling to work again every time this happens. Would the issue be the same as reported in this bug report?
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: