480506 - test_offsets.xul failed with some GNOME themes

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Ginn Chen]

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.

Comment 1

[Ginn Chen]

Attachment: patch

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.

Comment 2

[Ginn Chen]

Attachment: patch

missed a break in last patch

Comment 3

[Ginn Chen]

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.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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 5

[Ginn Chen]

Comment on attachment 364872 [details] [diff] [review]
patch

After looking at screen carefully, I found this patch is wrong.

Comment 6

[Ginn Chen]

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.

Comment 7

[Ginn Chen]

The bug is the test case.
In test_offsets.xul, scrollbox-test is a <scrollbar>, the border of <scrollbar> is not counted in "width"".

Comment 8

[Ginn Chen]

Attachment: patch

Comment 9

[Ginn Chen]

http://hg.mozilla.org/mozilla-central/rev/e3cde9cfb662