Uncollapsing a tree's scrollbar should not affect its min height

RESOLVED FIXED

Status

()

Core
XUL
RESOLVED FIXED
14 years ago
9 years ago

People

(Reporter: Brian Ryner (not reading), Assigned: Brian Ryner (not reading))

Tracking

({fixed1.8})

Trunk
fixed1.8
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

1.79 KB, patch
Brian Ryner (not reading)
: review+
Scott MacGregor
: approval1.8b5+
Details | Diff | Splinter Review
(Assignee)

Description

14 years ago
Spun off from bug 201379.

We need to come up with a real fix to ensure that uncollapsing a tree's
scrollbar does not cause the minimum height to increase.  If the minimum height
increases, there is the possibility of going into an overflow/underflow loop,
since more vertical space would be given to the tree if it's in a vbox with
other flexible children.

One solution would be to make the treebody always report a min-height that
equals the scrollbar's min height.  This may cause problems in cases like
autocomplete that want to create 1-row high trees.  We could further specify an
"overflow=hidden" or similar property on a tree that would cause the scrollbar
min height to _not_ be taken into account for these small trees (and they would,
of course, never uncollapse the scrollbar).

Another idea is to say that scrollbars must be able to become arbitrarily small.
 It would be up to the theme css and NativeTheme implementations to do something
reasonable at small sizes.  I'm actually not sure how you'd go about this for a
pure CSS theme.

Comment 1

14 years ago
Created attachment 131004 [details] [diff] [review]
Fix for Modern

You get strange results if you try to do this for Classic :-(

Comment 2

14 years ago
BTW, use attachment 84233 [details] to test the previous patch.
(Assignee)

Comment 3

14 years ago
Making it work in classic will definitely require changes to the native theme
code.  I just tested and I'm fairly sure gtk can draw scrollbars at an
arbitrarily small size (and since we're using the same drawing api as
GtkScrollbar, we should be able to as well).  I'm not sure whether that is
available through the theme API on Windows or not.  I'm also not sure what
happens to a Mac native scrollbar when you shrink it down (though it looks like
those may have a min size of 0 already...)

Comment 4

13 years ago
Comment on attachment 131004 [details] [diff] [review]
Fix for Modern

Or would you rather I put the flex in the XBL?
Attachment #131004 - Flags: superreview?(roc)
Attachment #131004 - Flags: review?(bryner)
Attachment #131004 - Flags: superreview?(roc) → superreview+
(Assignee)

Comment 5

13 years ago
Comment on attachment 131004 [details] [diff] [review]
Fix for Modern

Does the scrollbars-mini scrollbar[orient="horizontal"] > scrollbarbutton not
need the flex?

Comment 6

13 years ago
It will pick up the flex from the scrollbarbutton rule, the
scrollbar[orient="horizontal"] > scrollbarbutton only switches the dimensions.
(Assignee)

Comment 7

13 years ago
Comment on attachment 131004 [details] [diff] [review]
Fix for Modern

This may cause some interesting behavior when the scrollbar size is < 45px. 
Since the buttons and slider all have a flex of 1, it will attempt to divide up
the space equally, rather than the buttons taking what they need and giving the
rest to the slider.  I wonder if having a very high flex value on the buttons
would work better?  (Not that we always do reasonable things at small sizes
anyway)

Comment 8

13 years ago
Actually I quite like the "premature" shrinking button effect ;-) The slider
seems to be picking up the thumb's preferred height (width) but I can fix that
with a couple of extra styles in both CSS files:
slider { width: 0px; height: 15px; } 
slider[orient="vertical"] { width: 15px; height: 0px; }
Similarly in scrollbars-mini.css but with 11px instead of 15px.
(Assignee)

Comment 9

13 years ago
Comment on attachment 131004 [details] [diff] [review]
Fix for Modern

Are you planning to attach a new patch with those changes?

Comment 10

13 years ago
Created attachment 177249 [details] [diff] [review]
Fix slider size
Attachment #131004 - Attachment is obsolete: true
Attachment #177249 - Flags: review?(bryner)

Comment 11

12 years ago
*** Bug 299323 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

12 years ago
Attachment #131004 - Flags: review?(bryner)
(Assignee)

Updated

12 years ago
Attachment #177249 - Flags: review?(bryner) → review+

Comment 12

12 years ago
Fix checked in.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Comment 13

12 years ago
Comment on attachment 177249 [details] [diff] [review]
Fix slider size

Suite-only risk-free (as verified on irc) patch.
Attachment #177249 - Flags: approval1.8b5?

Comment 14

12 years ago
Comment on attachment 177249 [details] [diff] [review]
Fix slider size

+ this suite only bug, but please get it in ASAP. Thanks.
Attachment #177249 - Flags: approval1.8b5? → approval1.8b5+

Updated

12 years ago
Keywords: fixed1.8

Updated

9 years ago
Component: XP Toolkit/Widgets: Trees → XUL
QA Contact: shrir → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.