Merge lp:~larsu/overlay-scrollbar/fix-for-3.10 into lp:overlay-scrollbar

[Lars Karlitski]

Create native windows for the scrollbar overlay and tail

As of gtk 3.10, non-native windows are clipped to their widget's allocation. The hijacked scrollbar is given a 0-width or height when horizontal) allocation to make it disappear. Using native windows makes sure that the overlay and tail are drawn regardless.

Create native windows for the scrollbar overlay and tail

As of gtk 3.10, non-native windows are clipped to their widget's allocation. The hijacked scrollbar is given a 0-width or height when horizontal) allocation to make it disappear. Using native windows makes sure that the overlay and tail are drawn regardless.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:382
http://jenkins.qa.ubuntu.com/job/overlay-scrollbar-ci/10/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/overlay-scrollbar-trusty-amd64-ci/1
    SUCCESS: http://jenkins.qa.ubuntu.com/job/overlay-scrollbar-trusty-armhf-ci/1
    SUCCESS: http://jenkins.qa.ubuntu.com/job/overlay-scrollbar-trusty-i386-ci/1

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/overlay-scrollbar-ci/10/rebuild

review: Approve (continuous-integration)

[Sebastien Bacher (seb128)]

That looks fine to me and works fine with gtk 2, gtk 3.8 and gtk 3.10 on trusty. I'm only comment approving though, I would like somebody knowing the codebase a bit better acking the merge (or at least have a second review)

review: Approve

[Andrea Cimitan (cimi)]

Setting transient window is required at least for gtk2.
If you leave a window unfocused and click on its scrollbar, the window won't get focused without that.

The changes to use parent window kinda worries me for old Gtk too, but so far I didn't find bugs.

I saw you moved some bits in the realize function, but I think you still want to keep the old order... because you need to have events activated before adding the filter or it won't work.

Is setting the user data of the gdkwindow required in gtk2?

review: Needs Fixing

[lp:~larsu/overlay-scrollbar/fix-for-3.10]

383. By Lars Karlitski on 2013-11-29

Create windows with the widget's window as parent

Instead of the widget's parent window. This makes the code clearer, but
shouldn't be a functional difference, as scrollbars share a window with their
parent widget.

384. By Lars Karlitski on 2013-11-29

Only call gdk_window_ensure_native() for gtk3

Also, stop calling gdk_window_set_user_data() for gtk2.

385. By Lars Karlitski on 2013-11-29

Set overlay windows transient for the scroll bar window again

386. By Lars Karlitski on 2013-11-29

Don't set window filter func on the parent window

387. By Lars Karlitski on 2013-11-29

Revert rearranging scrollbar_realize

[Lars Karlitski (larsu)]

> Setting transient window is required at least for gtk2.
> If you leave a window unfocused and click on its scrollbar, the window won't
> get focused without that.

I've added it back in. The focus issue remains for gtk3 apps, though.

> The changes to use parent window kinda worries me for old Gtk too, but so far
> I didn't find bugs.
>
> I saw you moved some bits in the realize function, but I think you still want
> to keep the old order... because you need to have events activated before
> adding the filter or it won't work.

Sorry, they were from a previous refactor. I've reverted that.

> Is setting the user data of the gdkwindow required in gtk2?

It should be, but it wasn't before. This is unrelated to this patch and since it works without, let's not mingle the two.

[Andrea Cimitan (cimi)]

> > Setting transient window is required at least for gtk2.
> > If you leave a window unfocused and click on its scrollbar, the window won't
> > get focused without that.
>
> I've added it back in. The focus issue remains for gtk3 apps, though.
>
>
> > The changes to use parent window kinda worries me for old Gtk too, but so
> far
> > I didn't find bugs.
> >
> > I saw you moved some bits in the realize function, but I think you still
> want
> > to keep the old order... because you need to have events activated before
> > adding the filter or it won't work.
>
> Sorry, they were from a previous refactor. I've reverted that.
>
>
> > Is setting the user data of the gdkwindow required in gtk2?
>
> It should be, but it wasn't before. This is unrelated to this patch and since
> it works without, let's not mingle the two.

Thanks, let's approve one change at a time, this code is complex and might be easy to add regressions without realising :)

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:387
http://jenkins.qa.ubuntu.com/job/overlay-scrollbar-ci/11/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/overlay-scrollbar-trusty-amd64-ci/2
    SUCCESS: http://jenkins.qa.ubuntu.com/job/overlay-scrollbar-trusty-armhf-ci/2
    SUCCESS: http://jenkins.qa.ubuntu.com/job/overlay-scrollbar-trusty-i386-ci/2

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/overlay-scrollbar-ci/11/rebuild

review: Approve (continuous-integration)

[Sebastien Bacher (seb128)]

Works fine, tested on trusty with gtk2 (xchat), gtk3 3.8.7 and 3.10.5

review: Approve

=== modified file 'os/os-bar.c'
--- os/os-bar.c	2013-07-17 13:43:37 +0000
+++ os/os-bar.c	2013-11-29 13:57:08 +0000
@@ -715,6 +715,11 @@
                                             GDK_WA_VISUAL | GDK_WA_COLORMAP);
 #endif
 
+#ifdef USE_GTK3
+  gdk_window_ensure_native (priv->tail_window);
+  gtk_widget_register_window (priv->parent, priv->tail_window);
+#endif
+
   g_object_ref_sink (priv->tail_window);
 
   gdk_window_set_transient_for (priv->tail_window,
@@ -738,6 +743,11 @@
                                      GDK_WA_VISUAL | GDK_WA_COLORMAP);
 #endif 
 
+#ifdef USE_GTK3
+  gdk_window_ensure_native (priv->bar_window);
+  gtk_widget_register_window (priv->parent, priv->bar_window);
+#endif
+
   g_object_ref_sink (priv->bar_window);
 
   gdk_window_set_transient_for (priv->bar_window,
@@ -751,6 +761,8 @@
                                          gdk_region_new (),
 #endif
                                          0, 0);
+
+  mask_tail (bar);
 }
 
 /**