566154 - text missing from preferences font size pulldown.

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[George R. Goffe]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a5pre) Gecko/20100515 Minefield/3.7a5pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a5pre) Gecko/20100515 Minefield/3.7a5pre

In the preferences main panel to the right of "Fonts & Colors" is a pulldown menu that displays font sizes. There are no visible fonts listed although their slots in the pulldown are present and take effect if selected. You just can't read them.

The same is true in the "Advanced" section in the "Monospace" pulldown.

Reproducible: Always

Steps to Reproduce:
1.Edit -> Preferences
2.Left button click the font size spec
3.Text is missing in the pulldown
Actual Results:  
Text is missing in the pulldown

Expected Results:  
Text is visible in the pulldown

I don't know if this is REALLY a major function so that's what I selected.

Comment 1

[Ria Klaassen (not reading all bugmail)]

Confirmed for Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a5pre) Gecko/20100516 Minefield/3.7a5pre

Comment 2

[Ria Klaassen (not reading all bugmail)]

This is probably caused by one of Neil Deakin's bugs on 13 May 2010:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=80ae52a7e188&tochange=8e79b68f2d6c

Comment 3

[Alice0775 White]

http://hg.mozilla.org/mozilla-central/rev/b0691a6db8d3
causes this problem.

Comment 4

[Alice0775 White]

Attachment: testcase

Comment 7

[Neil Deakin]

This issue is caused because the size of the scrollbar isn't being accounted for when calculating the size that the menulist should be. The old way, before 562740, was to just extend the width of the popup so that the scrollbar extended outside of the button area, which looks odd.

An alternative is to unconditionally expand the width of the menulist button by the width of a scrollbar.

Comment 8

[Alice0775 White]

Attachment: testcase2

(In reply to comment #7)
> This issue is caused because the size of the scrollbar isn't being accounted
> for when calculating the size that the menulist should be. The old way, before
> 562740, was to just extend the width of the popup so that the scrollbar
> extended outside of the button area, which looks odd.
> 
testcase2
In case of old way (Firegox3.6.6), 
If browser is maximized, the scroll bar will overflow horizontally. 
The pull-down should take a width of the screen into account.

> An alternative is to unconditionally expand the width of the menulist button by
> the width of a scrollbar.
In this case, the layout of xul parts will be destroyed. and  it will flicker whenever I display pull-down.

Comment 9

[Neil Deakin]

Attachment: an implementation of the above for testing

This patch extends the menulist by the width of a scrollbar, but as suggested, it will extend the size possibly too much in some tight UI cases. But I think it may be the way to go.

Comment 10

[[e-mail]]

Comment on attachment 456882 [details] [diff] [review]
an implementation of the above for testing

This is great* but of course the themes will need to have their huge end padding (why 30px?) cut down to size.

Comment 11

[Neil Deakin]

(In reply to comment #10)

> This is great* but of course the themes will need to have their huge end
> padding (why 30px?) cut down to size.

What padding are you referring to?

Comment 12

[[e-mail]]

http://mxr.mozilla.org/mozilla-central/search?string=30px&find=menu.css

Comment 15

[Neil Deakin]

Attachment: fix

I included the general menulist test from bug 530504.

Comment 16

[[e-mail]]

Comment on attachment 464500 [details] [diff] [review]
fix

>   padding-top: 1px;
>-  -moz-padding-end: 30px;
>+  -moz-padding-end: 5px;
>   padding-bottom: 1px;
>   -moz-padding-start: 5px;
Nit: pinstripe/winstripe can be shortened to padding: 1px 5px;

>   padding-top: 1px;
>   padding-bottom: 1px;
>   -moz-padding-start: 1px;
>-  -moz-padding-end: 30px;
>+  -moz-padding-end: 1px;
And gnomestripe is even simpler: padding: 1px;

Comment 18

[Jim]

Attachment: Firefox 4 Beta 4 Default Font Dialog

I can confirm this is occurring in Firefox 4 Beta 4. Attached is a screen capture of the default font size drop-down box showing blank white instead of font sizes.

Comment 19

[[e-mail]]

Yes, i just checked , Jim is right about it, the drop down list is still missing in Firefox 4.0 Beta 4

Comment 21

[Neil Deakin]

Attachment: add comments ans fix an issue on Linux test

Comment 22

[[e-mail]]

(In reply to comment #16)
> Nit: pinstripe/winstripe can be shortened to padding: 1px 5px;
Sorry, I meant faststripe of course.

Comment 23

[Neil Deakin]

Attachment: update padding in theme as well

Comment 25

[Bill Gianopoulos [:WG9s]]

Attachment: screenshot

With this patch included, under linux, the currently selected value fails to display in the select boxes.

Changing the padding to "1px 5px" in gnomestripe to match winstripe and pinstripe corrects the issue.

Comment 26

[Dietrich Ayala (:dietrich)]

removing checkin-needed since the only r+d patch is obsolete. neil, can you address comment #25 and then get re-review or carry-over as necessary?

blocking+, this is pretty broken.

Comment 27

[[e-mail]]

(In reply to comment #25)
> Changing the padding to "1px 5px" in gnomestripe to match winstripe and
> pinstripe corrects the issue.
pmstripe might need to be changed too.

I wouldn't like to authoritatively claim that "1px 5px" is correct for pm/gnomestripe, but that's a ui decision and doesn't affect my review.

Comment 28

[Bill Gianopoulos [:WG9s]]

(In reply to comment #27)
> I wouldn't like to authoritatively claim that "1px 5px" is correct for
> pm/gnomestripe, but that's a ui decision and doesn't affect my review.

That was my thought as well.  For what it is worth, I tried leaving the 1px padding on the left and adding just enough padding on the right to avoid the issue and that just ended up with the selected font size being enough off center in the box and far away form the selector arrows to just look wrong.

One thing we don;t know is if the 1px on the left was ever a deliberate choice or was a choice made to winstripe after gnomestripe diverged that was never carried forward.

Sounds like maybe a ui review form the gnomestripe team is in order?

Comment 30

[Neil Deakin]

Attachment: just use the same padding for all platforms

Comment 31

[Bill Gianopoulos [:WG9s]]

(In reply to comment #30)
> Created attachment 473053 [details] [diff] [review]
> just use the same padding for all platforms

This makes sense to me.  Fix the issue so that at least the preferences work for all platforms and alert the Front end default theme owners about what was changed and which menus they should look at to make sure things they need to tweak to get the look they desire does not break the ability to set font preferences.  Otherwise we end up with a preference menu system that really does not work on any platform.

Comment 32

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/60f3736764d2

Comment 33

[u88484]

Verified fixed

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100916
Firefox/4.0b7pre