Closed Bug 218453 Opened 17 years ago Closed 15 years ago

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

Categories

(Core :: XUL, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bryner, Assigned: bryner)

References

Details

(Keywords: fixed1.8)

Attachments

(1 file, 1 obsolete file)

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.
Attached patch Fix for Modern (obsolete) — Splinter Review
You get strange results if you try to do this for Classic :-(
BTW, use attachment 84233 [details] to test the previous patch.
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 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+
Comment on attachment 131004 [details] [diff] [review]
Fix for Modern

Does the scrollbars-mini scrollbar[orient="horizontal"] > scrollbarbutton not
need the flex?
It will pick up the flex from the scrollbarbutton rule, the
scrollbar[orient="horizontal"] > scrollbarbutton only switches the dimensions.
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)
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.
Comment on attachment 131004 [details] [diff] [review]
Fix for Modern

Are you planning to attach a new patch with those changes?
Attached patch Fix slider sizeSplinter Review
Attachment #131004 - Attachment is obsolete: true
Attachment #177249 - Flags: review?(bryner)
*** Bug 299323 has been marked as a duplicate of this bug. ***
Attachment #131004 - Flags: review?(bryner)
Attachment #177249 - Flags: review?(bryner) → review+
Fix checked in.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
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 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+
Keywords: fixed1.8
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.