text missing from preferences font size pulldown.

VERIFIED FIXED in mozilla2.0b7

Status

()

--
major
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: grgoffe, Assigned: enndeakin)

Tracking

({regression})

Trunk
mozilla2.0b7
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

Attachments

(5 attachments, 4 obsolete attachments)

(Reporter)

Description

8 years ago
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.
Confirmed for Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a5pre) Gecko/20100516 Minefield/3.7a5pre
Status: UNCONFIRMED → NEW
Component: General → Preferences
Ever confirmed: true
OS: Linux → All
QA Contact: general → preferences
Hardware: x86 → All
Version: unspecified → Trunk
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
Component: Preferences → XP Toolkit/Widgets: Menus
Product: Firefox → Core
QA Contact: preferences → xptoolkit.menus

Comment 3

8 years ago
http://hg.mozilla.org/mozilla-central/rev/b0691a6db8d3
causes this problem.
Blocks: 562740
Keywords: regression

Comment 4

8 years ago
Created attachment 445823 [details]
testcase
Duplicate of this bug: 570093

Updated

8 years ago
Duplicate of this bug: 573744
(Assignee)

Comment 7

8 years ago
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

8 years ago
Created attachment 456088 [details]
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.
(Assignee)

Comment 9

8 years ago
Created attachment 456882 [details] [diff] [review]
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.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Attachment #456882 - Flags: feedback?(neil)
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.
Attachment #456882 - Flags: feedback?(neil) → feedback+
(Assignee)

Comment 11

8 years ago
(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?

Updated

8 years ago
Duplicate of this bug: 582925

Updated

8 years ago
Duplicate of this bug: 583938
(Assignee)

Comment 15

8 years ago
Created attachment 464500 [details] [diff] [review]
fix

I included the general menulist test from bug 530504.
Attachment #456882 - Attachment is obsolete: true
Attachment #464500 - Flags: review?(neil)
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;
Attachment #464500 - Flags: review?(neil) → review+

Updated

8 years ago
Duplicate of this bug: 589173

Comment 18

8 years ago
Created attachment 470675 [details]
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.
Yes, i just checked , Jim is right about it, the drop down list is still missing in Firefox 4.0 Beta 4
(Assignee)

Updated

8 years ago
Duplicate of this bug: 591736
(Assignee)

Comment 21

8 years ago
Created attachment 470872 [details] [diff] [review]
add comments ans fix an issue on Linux test
Attachment #464500 - Attachment is obsolete: true
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
(In reply to comment #16)
> Nit: pinstripe/winstripe can be shortened to padding: 1px 5px;
Sorry, I meant faststripe of course.
(Assignee)

Comment 23

8 years ago
Created attachment 471104 [details] [diff] [review]
update padding in theme as well
Attachment #470872 - Attachment is obsolete: true
Duplicate of this bug: 593639
blocking2.0: --- → ?
Created attachment 472261 [details]
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.
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.
blocking2.0: ? → betaN+
Keywords: checkin-needed
(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.
(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?

Updated

8 years ago
Duplicate of this bug: 594283
(Assignee)

Comment 30

8 years ago
Created attachment 473053 [details] [diff] [review]
just use the same padding for all platforms
Attachment #473053 - Flags: review+
(Assignee)

Updated

8 years ago
Attachment #471104 - Attachment is obsolete: true
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
(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

8 years ago
http://hg.mozilla.org/mozilla-central/rev/60f3736764d2
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b7

Comment 33

8 years ago
Verified fixed

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

Updated

8 years ago
Depends on: 623922
You need to log in before you can comment on or make changes to this bug.