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 686881 - Reserve scrollbars allocation when policy = automatic
Reserve scrollbars allocation when policy = automatic
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: st
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 676635 692456 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2012-10-25 17:56 UTC by Giovanni Campagna
Modified: 2013-01-24 20:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
StScrollView: add new policy for invisibile but allocated scrollbars (20.48 KB, patch)
2012-10-25 17:57 UTC, Giovanni Campagna
none Details | Review
scroll-view: reserve scrollbar allocation when policy is automatic (1.07 KB, patch)
2013-01-24 16:33 UTC, Cosimo Cecchi
none Details | Review
scroll-view: reserve scrollbar allocation when policy is automatic (2.30 KB, patch)
2013-01-24 20:04 UTC, Cosimo Cecchi
committed Details | Review
appDisplay: set hscrollbar_policy = NEVER on the category view actor (1.03 KB, patch)
2013-01-24 20:04 UTC, Cosimo Cecchi
committed Details | Review

Description Giovanni Campagna 2012-10-25 17:56:46 UTC
If you look very closely at the animation that happens when you click on a name in the login screen, you'll notice that the item is slightly shrunk. This is particularly noticeable if you click cancel, as it widens again.

Turns out this is a problem with the scrollbar: the scrollview requests size for the vertical scrollbar at all times, but the size of what it requests becomes 0 if the scrollbar is not visible, whereas if the scrollbar is visible but not needed the actor is allocated the entire space (including the requested scrollbar width).
To keep the animation smooth, what we want for the login screen instead is to always keep the scrollbar visible (in the clutter sense - we don't actually paint it), and to allocate the actor as if the scrollbar was actually there.

I now this is a minor bug, and the patch is not very small because I had to introduce a new policy enumeration for this particular case, but it really bugs me.
Comment 1 Giovanni Campagna 2012-10-25 17:57:13 UTC
Created attachment 227293 [details] [review]
StScrollView: add new policy for invisibile but allocated scrollbars

In certain situations, such as the login screen, we need the StScrollView
to have constant width, even if the height (and therefore the need for
vertical scrollbars) changes. To allow that, always request the right size
for scrollbars, and allocate them even if invisible if the policy says so.
This moves away from Gtk.PolicyType, although the new enumeration has the
same values, so it should be backwards compatible.
Comment 2 Jean-François Fortin Tam 2012-10-31 01:13:11 UTC
Giovanni, this is actually a duplicate of bug #676635 - has your patch been committed or does it take into account the scenarios from that bug?
Comment 3 Jasper St. Pierre (not reading bugmail) 2012-10-31 01:16:51 UTC
(In reply to comment #1)
> Created an attachment (id=227293) [details] [review]
> StScrollView: add new policy for invisibile but allocated scrollbars
> 
> In certain situations, such as the login screen, we need the StScrollView
> to have constant width, even if the height (and therefore the need for
> vertical scrollbars) changes. To allow that, always request the right size
> for scrollbars, and allocate them even if invisible if the policy says so.
> This moves away from Gtk.PolicyType, although the new enumeration has the
> same values, so it should be backwards compatible.

I don't see why we *shouldn't* always do this. What other cases are there for scroll views where we shouldn't allocate some extra padding?
Comment 4 Giovanni Campagna 2012-11-01 16:10:58 UTC
@Jean-François:
Sorry, I didn't know of bug 676635.
In any case, that bug is a slightly different one, and was caused by hiding the user name after clicking on it, which no longer happens. I guess you can close that obsolete.

@Jasper:
I don't know. IMHO, just changing GTK_POLICY_AUTOMATIC is less likely to go noticed by the right people, and would expose us to regressions.
As for specific cases, I think for example in the chat and notification scroll views it makes sense to use all the available space.
Comment 5 Giovanni Campagna 2012-11-24 18:08:45 UTC
*** Bug 676635 has been marked as a duplicate of this bug. ***
Comment 6 Giovanni Campagna 2013-01-04 18:09:21 UTC
So, what are we going to do here? Should I just change the behaviour of GTK_POLICY_AUTOMATIC and see what breaks?
Comment 7 Jasper St. Pierre (not reading bugmail) 2013-01-18 18:38:30 UTC
(In reply to comment #6)
> So, what are we going to do here? Should I just change the behaviour of
> GTK_POLICY_AUTOMATIC and see what breaks?

Let's try, yes.
Comment 8 Cosimo Cecchi 2013-01-24 16:29:51 UTC
*** Bug 692456 has been marked as a duplicate of this bug. ***
Comment 9 Cosimo Cecchi 2013-01-24 16:33:21 UTC
Created attachment 234312 [details] [review]
scroll-view: reserve scrollbar allocation when policy is automatic

--

Implements the automatic solution proposed by Jasper.
Comment 10 Jasper St. Pierre (not reading bugmail) 2013-01-24 16:38:26 UTC
Review of attachment 234312 [details] [review]:

Let's look at the cases where we have automatic scroll views and see what changes for each of them.

* Message tray notifications.
* Applications view.
* Submenus (like in the network menu)
* Search.
* Looking Glass
* Gdm Session List
* Inhibitors List in the Log Out dialog

How does this look?

and I think that's it?
Comment 11 Cosimo Cecchi 2013-01-24 17:17:53 UTC
I think in each of those cases it should be either an improvement, or not change anything (e.g. because the content is not center-aligned, like in the Applications view).
Comment 12 Cosimo Cecchi 2013-01-24 20:04:22 UTC
Created attachment 234325 [details] [review]
scroll-view: reserve scrollbar allocation when policy is automatic

--

New version; fixes a leftover from the previous assumptions in get_preferred_height()
Comment 13 Cosimo Cecchi 2013-01-24 20:04:28 UTC
Created attachment 234326 [details] [review]
appDisplay: set hscrollbar_policy = NEVER on the category view actor
Comment 14 Jasper St. Pierre (not reading bugmail) 2013-01-24 20:06:49 UTC
Review of attachment 234325 [details] [review]:

OK.
Comment 15 Jasper St. Pierre (not reading bugmail) 2013-01-24 20:06:59 UTC
Review of attachment 234326 [details] [review]:

OK.
Comment 16 Jasper St. Pierre (not reading bugmail) 2013-01-24 20:09:04 UTC
Cosimo and I went over all the scroll views set to AUTOMATIC we found in the code, and none of them broke, besides the category list. This has been fixed in two ways: the code that did incorrect allocations was fixed, and we ensure that we never show a horizontal scrollbar on the category list.

We couldn't test the inhibitor list in the end session dialog because it seems the inhibitor list is broken, but it should be the same as the login dialog.
Comment 17 Cosimo Cecchi 2013-01-24 20:10:21 UTC
Attachment 234325 [details] pushed as 426581e - scroll-view: reserve scrollbar allocation when policy is automatic
Attachment 234326 [details] pushed as 0769608 - appDisplay: set hscrollbar_policy = NEVER on the category view actor