Merge lp:~townsend/compiz/fix-gtk-mouse-scrolling into lp:compiz/0.9.11

[Christopher Townsend]

Fix issue where Compiz passively grabs all mouse buttons which severely affects scrolling in Gtk apps. Now only unconditionally grab buttons 1-3 for all windows except the desktop window and only grab other buttons when there is a modifier attached to it.

Fix issue where Compiz passively grabs all mouse buttons which severely affects scrolling in Gtk apps. Now only unconditionally grab buttons 1-3 for all windows except the desktop window and only grab other buttons when there is a modifier attached to it.

[lp:~townsend/compiz/fix-gtk-mouse-scrolling]

3848. By Christopher Townsend on 2014-04-02

Fix indentation formatting.

[Brandon Schaefer (brandontschaefer)]

Hopefully we'll never need to grab the wheels!

LGTM

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3848
http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-ci/52/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-trusty-amd64-ci/52
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-trusty-armhf-ci/52
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-0.9.11-trusty-i386-ci/52

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/compiz-0.9.11-ci/52/rebuild

review: Approve (continuous-integration)

[Marco Trevisan (Treviño) (3v1n0)]

Cool, thanks!

review: Approve

[Sam Spilsbury (smspillaz)]

Please manually test this against links in gtk+ applications to check for
no regressions.
On Apr 3, 2014 5:34 AM, "Christopher Townsend" <
<email address hidden>> wrote:

> Christopher Townsend has proposed merging
> lp:~townsend/compiz/fix-gtk-mouse-scrolling into lp:compiz.
>
> Commit message:
> Fix issue where Compiz passively grabs all mouse buttons which severely
> affects scrolling in Gtk apps. Now only unconditionally grab buttons 1-3
> for all windows except the desktop window and only grab other buttons when
> there is a modifier attached to it.
>
> Requested reviews:
> Compiz Maintainers (compiz-team)
> Related bugs:
> Bug #1171342 in Compiz: "mouse scroll wheel not working in gedit &
> System Monitor"
> https://bugs.launchpad.net/compiz/+bug/1171342
> Bug #1184159 in Compiz: "[saucy] scrolling with a touchpad is jerky with
> bindings set in compiz for Desktop-based Viewport Switching"
> https://bugs.launchpad.net/compiz/+bug/1184159
> Bug #1200829 in Compiz: "Regression: Enabling typical bindings in
> "Desktop-based Viewport Switching" breaks scrollwheel scrolling in some
> windows with a usb mouse on a laptop"
> https://bugs.launchpad.net/compiz/+bug/1200829
> Bug #1240957 in Compiz: "Scrolling behaviour and window focus has
> changed and is inconsistent"
> https://bugs.launchpad.net/compiz/+bug/1240957
>
> For more details, see:
>
> https://code.launchpad.net/~townsend/compiz/fix-gtk-mouse-scrolling/+merge/213930
>
> Fix issue where Compiz passively grabs all mouse buttons which severely
> affects scrolling in Gtk apps. Now only unconditionally grab buttons 1-3
> for all windows except the desktop window and only grab other buttons when
> there is a modifier attached to it.
> --
>
> https://code.launchpad.net/~townsend/compiz/fix-gtk-mouse-scrolling/+merge/213930
> Your team Compiz Maintainers is requested to review the proposed merge of
> lp:~townsend/compiz/fix-gtk-mouse-scrolling into lp:compiz.
>
> === modified file 'src/screen.cpp'
> --- src/screen.cpp 2014-03-27 18:40:07 +0000
> +++ src/screen.cpp 2014-04-02 21:33:21 +0000
> @@ -3413,6 +3413,17 @@
>
> void cps::GrabManager::updatePassiveButtonGrabs(Window serverFrame)
> {
> + CompWindow *window = NULL;
> +
> + foreach (CompWindow *w, screen->windows ())
> + {
> + if (w->frame () == serverFrame)
> + {
> + window = w;
> + break;
> + }
> + }
> +
> /* Grab only we have bindings on */
> foreach (ButtonGrab &bind, buttonGrabs)
> {
> @@ -3427,6 +3438,12 @@
> if (ignore & ~modHandler->ignoredModMask ())
> continue;
>
> + if (window &&
> + !(window->type () & CompWindowTypeDesktopMask) &&
> + bind.button > 3 &&
> + !mods)
> + continue;
> +
> XGrabButton (screen->dpy(),
> bind.button,
> mods | ignore,
>
> === modified file 'src/window.cpp'
> --- src/window.cpp 2014-03-27 18:40:07 +0000
> +++ src/window.cpp 2014-04-02 21:33:21 +0000
> @@ -5620,20 +5620,21 @@
> }
> }
>
> - if (onlyActions)
> + if...

[Christopher Townsend (townsend)]

Hey Sam,

Thanks for the suggestion. Could you give a bit more detail on what you mean by "links in gtk+ applications"? I take that as links to URLs that one clicks on to open the page in a web browser, but you might mean something else completely:)

[Sam Spilsbury (smspillaz)]

Hey Chris,

Yeah, sorry, I was on my phone when I saw that - If you open up
gtk3-demo in the gtk+ source tree, and then open the "Links" demo,
just check if the links are clickable. I put that code in to fix the
behaviour of links in gtk+ and I remember it was quite sensitive to
the way passive grabs worked.

On Thu, Apr 3, 2014 at 8:37 PM, Christopher Townsend
<email address hidden> wrote:
> Hey Sam,
>
> Thanks for the suggestion. Could you give a bit more detail on what you mean by "links in gtk+ applications"? I take that as links to URLs that one clicks on to open the page in a web browser, but you might mean something else completely:)
> --
> https://code.launchpad.net/~townsend/compiz/fix-gtk-mouse-scrolling/+merge/213930
> Your team Compiz Maintainers is subscribed to branch lp:compiz.

--
Sam Spilsbury

[Christopher Townsend (townsend)]

Hey Sam,

Thanks for the additional info. Yep, the Links demo works fine with this fix.

[Sam Spilsbury (smspillaz)]

Excellent - that was the only thing I was going to mention :)

On Thu, Apr 3, 2014 at 10:21 PM, Christopher Townsend
<email address hidden> wrote:
> Hey Sam,
>
> Thanks for the additional info. Yep, the Links demo works fine with this fix.
> --
> https://code.launchpad.net/~townsend/compiz/fix-gtk-mouse-scrolling/+merge/213930
> Your team Compiz Maintainers is subscribed to branch lp:compiz.

--
Sam Spilsbury

=== modified file 'src/screen.cpp'
--- src/screen.cpp	2014-03-27 18:40:07 +0000
+++ src/screen.cpp	2014-04-02 21:45:56 +0000
@@ -3413,6 +3413,17 @@
 
 void cps::GrabManager::updatePassiveButtonGrabs(Window serverFrame)
 {
+    CompWindow *window = NULL;
+
+    foreach (CompWindow *w, screen->windows ())
+    {
+	if (w->frame () == serverFrame)
+	{
+	    window = w;
+	    break;
+	}
+    }
+
     /* Grab only we have bindings on */
     foreach (ButtonGrab &bind, buttonGrabs)
     {
@@ -3427,6 +3438,12 @@
 	    if (ignore & ~modHandler->ignoredModMask ())
 		continue;
 
+	    if (window &&
+		!(window->type () & CompWindowTypeDesktopMask) &&
+		bind.button > 3 &&
+		!mods)
+		continue;
+
 	    XGrabButton (screen->dpy(),
 			 bind.button,
 			 mods | ignore,
 
=== modified file 'src/window.cpp'
--- src/window.cpp	2014-03-27 18:40:07 +0000
+++ src/window.cpp	2014-04-02 21:45:56 +0000
@@ -5620,20 +5620,21 @@
 	}
     }
 
-    if (onlyActions)
+    if (onlyActions || priv->type & CompWindowTypeDesktopMask)
 	screen->updatePassiveButtonGrabs(serverFrame);
     else
     {
-	/* Grab everything */
-	XGrabButton (screen->dpy (),
-		     AnyButton,
-		     AnyModifier,
-		     serverFrame, false,
-		     ButtonPressMask | ButtonReleaseMask | ButtonMotionMask,
-		     GrabModeSync,
-		     GrabModeAsync,
-		     None,
-		     None);
+	/* Only grab buttons 1-3 */
+	for (int i = 1; i <= 3; i++)
+	    XGrabButton (screen->dpy (),
+			 i,
+			 AnyModifier,
+			 serverFrame, false,
+			 ButtonPressMask | ButtonReleaseMask | ButtonMotionMask,
+			 GrabModeSync,
+			 GrabModeAsync,
+			 None,
+			 None);
     }
 }