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 629778 - Scrolled window does not behave properly with height-for-width contents
Scrolled window does not behave properly with height-for-width contents
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkScrolledWindow
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2010-09-15 16:36 UTC by Tristan Van Berkom
Modified: 2010-10-12 08:13 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Gif showing how it reproduces on my desktop (714.16 KB, image/gif)
2010-09-16 03:17 UTC, Tristan Van Berkom
  Details
Patch to address this problem (15.36 KB, patch)
2010-10-05 06:25 UTC, Tristan Van Berkom
accepted-commit_now Details | Review

Description Tristan Van Berkom 2010-09-15 16:36:34 UTC
To reproduce:
  - startup ./tests/testwrapbox
  - close both control expanders (note now we have 
    available width for 4 columns)
  - resize (shorten) the window vertically until a vertical scrollbar
    is needed (note now there is space for only 3 columns again)
  - now resize the window vertically to make it larger.

Bug: the vertical scrollbar will remain visible until stretched out
     completely with still only 3 columns showing.

Expected behaviour: The vertical scrollbar should dissapear as soon
as the required height for the full allocation width (without any 
vertical scrollbar) is small enough to not need a scrollbar.
Comment 1 Tristan Van Berkom 2010-09-15 16:41:32 UTC
Note, bug 611740 describes in depth how the issue was addressed so
far in St (gnome-shell clutter based actors).
Comment 2 Matthias Clasen 2010-09-15 17:00:58 UTC
The scrollbar disappears just fine here :-(
Comment 3 Javier Jardón (IRC: jjardon) 2010-09-15 18:33:26 UTC
I can't reproduce here neither
Comment 4 Tristan Van Berkom 2010-09-16 03:17:26 UTC
Created attachment 170389 [details]
Gif showing how it reproduces on my desktop

Really staggeringly odd that I can reproduce this and both
of you cant, this morning its still reproducible (after letting
me computer sleep)...
Comment 5 Matthias Clasen 2010-09-17 14:35:34 UTC
Wait, thats exactly what I am seeing. What is wrong with that ?
I thought the scrollbar failed to disappear ?
Comment 6 Matthias Clasen 2010-09-17 14:37:02 UTC
Oh, I see what you are saying. You expect the wrapbox to flip back to 4 columns, making the content fit and the scrollbar unnecessary
Comment 7 Tristan Van Berkom 2010-09-18 05:23:43 UTC
Right, the screwyness is a bit more obvious if you set the min-items to 2
instead of 3 and make the window a bit less wide... then its a bit more obvious 
that the vscrollbar is unneeded for a long duration while resizing the window.

But basically, the scrollbars should never be visible when the content
fits the allocation.

Thanks for looking at this again, I was starting to think I was going mad
(and recompiled all gtk+ dependencies ;-)).
Comment 8 Tristan Van Berkom 2010-10-05 06:25:55 UTC
Created attachment 171739 [details] [review]
Patch to address this problem

From descriptive local git commit log:

    Added logic to GtkScrolledWindow when allocating height-for-width children.
    
    This patch makes the scrolled window reconsider allocating the child
    the full width or height (depending on the child's request mode) without
    a scrollbar. For instance when the child is height-for-width; the child
    will first be tested if the content's height for full allocated width
    (without a vscrollbar) will allow the contents height for that width
    to fit the allocated height.
    
    Patch is a simplified version of code inspected in st-scroll-view.c.
    Note that this patch assumes children will begin to scroll only after
    reaching their minimum size; adding a property to the future
    GtkScrollableIface to decide whether to scroll-to-minimum or 
    scroll-to-natural will effect this code (it should then reconsider 
    whether the child will scroll below the natural size instead of 
    the minimum).
    
    Patch also offloads border-width to the parent GtkContainerClass using
    gtk_container_class_handle_border_width().
Comment 9 Tristan Van Berkom 2010-10-05 06:30:29 UTC
Note that this patch maintains the old logic in place used to detect
infinite recursion loops; this detection still works when used with
GtkWrapBox as the height-for-width apis work correctly and return
synchronous results.

However the problem of asynchronously resizing height-for-width widgets
is still left unadressed and should be considered a separate issue.
(cases like GtkTextView or height-for-width treeviews leave the possibility
where a new height calculated in an g_idle callback will throw the
infinite loop detection off completely).

I think though, that these asynchronous cases may be corner cases in
the end, maybe it will be wiser to handle the possible infinite recursion
from those widgets themselves instead of from the scrolled-window code
(not sure yet how or if its possible to detect this asynchronous looping
without more context of the content; hence why it might be wiser to address
it from textview/treeview code).
Comment 10 Matthias Clasen 2010-10-12 04:14:40 UTC
Review of attachment 171739 [details] [review]:

Can we commit this in two parts ? First the borderwidth handling, then the hfw addition.

::: gtk/gtkscrolledwindow.c
@@ +1456,3 @@
+		    }
+		}
+	      else /* priv->hscrollbar_policy != GTK_POLICY_AUTOMATIC */

the comment is wrong, should be priv->vscrollbar_policy != GTK_POLICY_AUTOMATIC
Comment 11 Tristan Van Berkom 2010-10-12 08:13:58 UTC
Applied patch separated into topics as requested (and fixed comments).