FFE: Provide support for dynamically loading the new overlay scrollbar feature

Bug #730740 reported by David Barth on 2011-03-07
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
gtk+2.0 (Ubuntu)
Wishlist
Unassigned

Bug Description

The https://launchpad.net/ayatana-scrollbar project provides an alternative scrollbar for GTK applications, to optimize for screen real-estate.

The feature requires a patch to GTK+2.0 to optionally load the replacement scrollbars.

This bug captures the request to integrate this patch and simplify the installation and use of the feature for users wishing to use it on stock Natty.

The scope of the request is for the inclusion of the GTK patch, not for the overlay scrollbar which remains as a loadable module.

The patch is up for review at http://bazaar.launchpad.net/~ayatana-scrollbar-team/ayatana-scrollbar/trunk/view/head:/misc/gtk%2B-2.0-maverick-use-overlay-scrollbar-from-module.patch

Sebastien Bacher (seb128) wrote :

the _gtk_scrolled_window_init() should be namespaced ubuntu since that's an ubuntu specific addition, the patch doesn't seem to be a real issue but doesn't seem

Changed in gtk+2.0 (Ubuntu):
importance: Undecided → Wishlist
Andrea Cimitan (cimi) wrote :

please note that, from the patch above, we need to update the patch for each ayatana scrollbar release. do we want this?

Sebastien Bacher (seb128) wrote :

(sorry clicked enter by error)
...but doesn't seem a clean way to add a such feature as well but rather a setup for demo, do we really need to that hack in natty?

Changed in gtk+2.0 (Ubuntu):
status: New → Confirmed
Loïc Molinari (loic.molinari) wrote :

Cimi:

That's not an issue, the series (major-minor number) is not going to be bumped for that cycle, so the GTK+ patch doesn't need to be updated for each release in the 0.1 series.

Sébastien:

From a technical point of view, I think that the clean way, which makes it potentially acceptable upstream, is to add that new widget directly inside GTK+ and add new symbols in GtkScrolledWindow to choose the scrollbar type to be used. But we've done that using a dynamically loaded module for this cycle because it reduces the GTK+ library size making it easier to fit on the CD, and it simplifies updating the ayatana-scrollbar library without implying a GTK+ update. Depending on the feedback, it will be easy to convert it to the clean way for Natty+1.

I'll update the patch adding the ubuntu prefix.

Martin Pitt (pitti) wrote :

FFe wise, is that really something that we need to squeeze into natty, given our limited resources which we should better spend fixing bugs, regressions, missing features (a11y, accessibility, keyboard navigation), and performance of Unity?

David Barth (dbarth) wrote :

The main resource (Cimi) can't really help with the rest of the unity issues. Cimi is dedicated to toolkit, theme and theme engine improvements for Natty.

I understand however that bugs triggered by this optional module should not impact the rest of the release. So we could update the gtk apport hook to add an 'overlay-scrollbar' tag to issues reported on applications where the LIBOVERLAY_SCROLLBAR env. variable was set. And put those outside of the release showstoppers list.

Sebastien Bacher (seb128) wrote :

still it seems that's not something natty will make use so what is the rational to justify spending work on that now rather than spending those efforts working on natty? it also seems that using a ppa would give extra flexibility to people working on the scrollbars and allow updates after natty is frozen

David Barth (dbarth) wrote :

Hereby requesting a review of the FFE request.

The gtk support patch is up for review at: https://bugs.launchpad.net/ayatana-scrollbar/+bug/740276 (see comment #2) along with an updated branch supporting whitelist, blacklist, and environment variable to control when the feature should be active.

A public testing campaign was run during the past weeks, with application compatibility results captured at: https://wiki.ubuntu.com/Ayatana/ScrollBars

The code for the feature and the gtk patch have been code reviewed per our standard development practices.

A very significant performance improvement was also made recently that suppresses slow downs noticed on particular apps (evolution for example). See https://bugs.launchpad.net/ayatana-scrollbar/+bug/728418

David Barth (dbarth) wrote :

Sorry for the confusion, please consider the patch as of comment #3 here https://bugs.launchpad.net/ayatana-scrollbar/+bug/740276/comments/3

It's been tested and validated with rev. 172 of the library (http://bazaar.launchpad.net/~ayatana-scrollbar-team/ayatana-scrollbar/trunk/revision/172) which we are submitting for upload into Natty as well.

Martin Pitt (pitti) wrote :

Setting back to NEW, as this hasn't been approved yet. As I already said, it's not something that I personally want to push, as we have enough to deal with in natty (release is in a month, and we still need to sort out crashers, how to handle the a11y regressions, compiz bugs, etc., and we haven't even started to address performance/boots peed regressions). I wouldn't like to see this squeezed into natty as well, especially not if we can only provide it for some applications (which will look quite inconsistent).

That being said, I expect Mark to exercise his SABDFL powers here and approve it himself. Subscribing him for the formal approval.

summary: - Provide support for dynamically loading the new overlay scrollbar
+ FFE: Provide support for dynamically loading the new overlay scrollbar
feature
Changed in gtk+2.0 (Ubuntu):
status: Confirmed → New
Martin Pitt (pitti) wrote :

Also, I'm worried the dlopen() hack and changing API without at least proposing this upstream (see Sebastien's concerns above).

Sebastien Bacher (seb128) wrote :

Ok, for the record I think the way the GTK patch is done (checking for a fixed path on disk to dlopen a library and override the gtk behaviour in a non documented way) is wrong and that we should better do it the right way next cycle than to land it in natty.
That said the patch doesn't seem it would be a stability issue for gtk if those are not used.

Speaking about the scrollbars themself I would also be in favor of landing something when it's ready, the current version is nice but works only on a selected set of applications and locales which is not really something which seems ready for primetime in a stable distribution.

That said if we decide to go for that, once the gtk patch is updated to correctly prefix the function it can be uploaded to natty

David Barth (dbarth) wrote :

It works on more than a fixed set of applications: the whitelist is meant to enable it by default, for a set of applications that we are testing more thoroughly. It should work for all gtk applications using gtk_scrolled_window, ie most of the gnome applications we know of.

The missing LTR support is because we want to test that fully as well. It should work reasonnably with LTR languages, but not with the level of visual quality we aim for.

Mark Shuttleworth (sabdfl) wrote :

Thanks all for comments and things to consider.

I'm weighing up:

 * that the patch to gtk itself is considered safe and stable, though ugly
 * that we have a whitelist mechanism to enable us to test and deploy this in selected apps

I'd ask that the patched gtk be uploaded as soon as the archive opens after beta1, together with the package for liboverlayscrollbar which should go into universe. An MIR should be filed for liboverlayscrollbar, pending the results of more widespread testing.

This initial upload of liboverlayscrollbar should be whitelisted on a core set of apps that Cimi is happy should work well. The result should be that overlay scrollbars Just Work on that set of apps, for people who apt-get liboverlayscrollbar. We will blog and encourage substantial testing of that package with those apps. I believe that apport hooks exist already to tag any crashes that include overlay scrollbars appropriately. We will watch very closely the number and nature of those crashers. During this testing period I would ask that Cimi and Seb or Didrocks have discretion to widen the whitelist, or enable the use of the overlay scrollbars in all supported apps, without further review.

Before beta2, we will take a decision about the main and CD inclusion of liboverlay scrollbar, and whether that will be whitelisted (i.e. only in selected apps) or turned-on-by-default.

Thanks all!
Mark

Martin Pitt (pitti) wrote :

Marking as approved then, as per Mark's comment above.

Changed in gtk+2.0 (Ubuntu):
status: New → Confirmed
David Barth (dbarth) wrote :

Additional information for the record.

The feature is currently enabled by default, with the applications listed at: http://bazaar.launchpad.net/~ayatana-scrollbar-team/ayatana-scrollbar/trunk/view/head:/os/os-utils.c#L57 It is based on the application compatibility list captured at https://wiki.ubuntu.com/Ayatana/ScrollBars#Application Compatibility based on user feedback.

An apport hook has also been provided (https://bugs.launchpad.net/ayatana-scrollbar/+bug/740278) to tag bug reports generated by systems where the ayatana-scrollbars are installed.

The GTK patch has been reworked to use an ubuntu_* prefix to better differentiate the code.

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package gtk+2.0 - 2.24.3-0ubuntu5

---------------
gtk+2.0 (2.24.3-0ubuntu5) natty; urgency=low

  * debian/patches/100_overlay_scrollbar_loading.patch
    - support for dynamically loading overlay scrollbars (LP: #730740)
 -- Ken VanDine <email address hidden> Wed, 30 Mar 2011 08:57:26 -0400

Changed in gtk+2.0 (Ubuntu):
status: Confirmed → Fix Released

Thanks all, glad to see the hooks land. We'll encourage folk to install
the liboverlayscrollbar package directly, and if apport and anecdotal
reports are positive, bring it in for even wider testing.

Sebastien Bacher (seb128) wrote :

seems that bug #740765 which is nautilus crashing when closing a window that we have been investigated for some days and pinged nautilus upstream about might be happening only when the scrollbars are installed, could someone check on that issue? note that the crasher has quite some duplicates

Sebastien Bacher (seb128) wrote :

see bug #748552 as well

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers