Closed Bug 480506 Opened 11 years ago Closed 11 years ago

test_offsets.xul failed with some GNOME themes

Categories

(Core :: Widget: Gtk, defect)

All
OpenSolaris
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: ginnchen+exoracle, Assigned: ginnchen+exoracle)

References

Details

Attachments

(1 file, 2 obsolete files)

Crux:
 scrollbox clientWidth got 37, expected 39
 scrollbox clientHeight got 28, expected 30

Glider
 scrollbox clientWidth got 33, expected 37
 scrollbox clientHeight got 53, expected 38 ( I didn't see horizon scrollbar in screen)

High Contrast
 scrollbox clientWidth got 36, expected 38
 scrollbox clientHeight got 58, expected 44 ( I didn't see horizon scrollbar in screen)

Low Contrast
 scrollbox clientWidth got 36, expected 38
 scrollbox clientHeight got 30, expected 32

Nimbus
 scrollbox clientWidth got 37, expected 39
 scrollbox clientHeight got 36, expected 38

Clearlooks, Glossy passed

I took a look with Nimbus theme, expected clientHeight is 2(paddingTop) + 47(height) + 2(paddingBottom) - 13(scrollbarHeight) = 38. But on screen, the scrollbar is 15px in height.
Attached patch patch (obsolete) — Splinter Review
1) trough_border is 2-direction not 4-direction.
2) I don't understand why we should use different code in GetMinimumWidgetSize() for NS_THEME_SCROLLBAR_THUMB_* and NS_THEME_SCROLLBAR_BUTTON_*.
It doesn't make sense to me.
3) 60x50 is a bit too small for some themes, e.g. GNOME High Contrast theme. Make it a bit larger.
Assignee: nobody → ginn.chen
Status: NEW → ASSIGNED
Attachment #364871 - Flags: review?(roc)
Attached patch patch (obsolete) — Splinter Review
missed a break in last patch
Attachment #364871 - Attachment is obsolete: true
Attachment #364872 - Flags: review?(roc)
Attachment #364871 - Flags: review?(roc)
I need to change the vbox size to 70x60 to get it work for "High Contrast Large Print" theme.
Perhaps we need to fix the calculation in js in case of scrollbar is not displayed.
But it is a minor issue.
The patch is OK except for the changes to thumb sizing. We should definitely be using min_slider_size not stepper_size there. The existing code is also making sure that the thumb can shrink to nothing if the scrollbar gets very short, and I think we should keep doing that unless we have a good reason not to.
Comment on attachment 364872 [details] [diff] [review]
patch

After looking at screen carefully, I found this patch is wrong.
Attachment #364872 - Attachment is obsolete: true
Attachment #364872 - Flags: review?(roc)
I gave the border wrong direction.
So actually my patch ignore the border.

The real problem here is gcs($"scrollbox-test"), "width") doesn't count border in .xul file, but it counts border in .html file.
The bug is the test case.
In test_offsets.xul, scrollbox-test is a <scrollbar>, the border of <scrollbar> is not counted in "width"".
Attached patch patchSplinter Review
Attachment #365169 - Flags: review?(roc)
http://hg.mozilla.org/mozilla-central/rev/e3cde9cfb662
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.