Closed Bug 393804 Opened 13 years ago Closed 12 years ago

Top level "View" drop down menu items are not aligned properly

Categories

(Toolkit :: XUL Widgets, defect, P2, minor)

x86
Windows XP
defect

Tracking

()

RESOLVED FIXED
mozilla1.9beta1

People

(Reporter: BoxerBoi76, Assigned: Gavin)

References

Details

(Keywords: regression)

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007082616 Minefield/3.0a8pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007082616 Minefield/3.0a8pre

Both "Status Bar" and "Full Screen" are not vertically aligned with the rest of the item under "View" in default "Minefield/Firefox" theme (3.0)

Reproducible: Always

Steps to Reproduce:
1) Launch current build of Minefield
2) Click "View" and observe the non-aligned items
Actual Results:  
1) Launch current build of Minefield
2) Click "View" and observe the non-aligned items

Expected Results:  
That all items in all drop down menus are vertically aligned.
Attached image Image showing issue...
Also Full Screen and in the File menu the Work Offline seem to have a problem.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Version: unspecified → Trunk
(In reply to comment #2)
> Also Full Screen 
> 
Like you said, sorry. :)

Ria,

I've also noticed at times that "Check for Updates" under Help has this issue when it's actually downloading an update.  In it's normal state it's off by a pixel or two IIRC, again much worse when it's actually downloading an update.

~B
Severity: trivial → minor
Component: Menus → XUL Widgets
Product: Firefox → Toolkit
QA Contact: menus → xul.widgets
1) "View-> Show Columns" has the same issue in Bookmarks Manager
2) The top three and bottom three options in "View-> Character Encoding->" in FF are slightly mis-aligned
3) There is more vertical spacing (padding?) in "Character Encoding->" in the mouseover hightlight.  Click "View-> Character Encoding-> Auto Detect-> Off" to see what I mean.  Actually, all of the submenus under "View" have issues

~B
Attached patch patch?Splinter Review
This is the patch Rob suggested when I asked him about this bug. r?him so that he can explain why it's correct :)
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Attachment #280388 - Flags: review?(vladimir)
Attachment #280388 - Flags: review?(tellrob)
Priority: -- → P2
Target Milestone: --- → mozilla1.9 M9
IIRC from bug 337771, this issue depends on the default system font: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/themes/winstripe/global/menu.css&rev=&cvsroot=/cvsroot&mark=109#107 (note the use of the font-_height_ dependent em unit for a width).

Changing that line to a fixed pixel width might have issues for very large default font sizes (which lead to enlarged check marks and bullets), though.
Well, seeing as how the patch that caused this bug forced the checkbox image to be 16px wide rather than 14px, the patch does seem reasonable (adding the same 2px to all other menu items)... do you think it's the wrong way to fix this?
Oh, that's indeed a change which has happened later on (in bug 355789). Still not sure whether that change was a good idea in the first place - but sure, give this simple fix a try.
Comment on attachment 280388 [details] [diff] [review]
patch?

This patch changes the left padding on text in menu items when there is no menu theme. It does not affect checkmenuitems, radiomenuitems or menuitems with images.
Attachment #280388 - Flags: review?(tellrob) → review+
Attachment #280388 - Flags: approval1.9?
Comment on attachment 280388 [details] [diff] [review]
patch?

a=bzbarsky
Attachment #280388 - Flags: approval1.9? → approval1.9+
mozilla/widget/src/windows/nsNativeThemeWin.cpp 	3.95
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
This looks ok with normal sized fonts.  But setting the Appearance Settings to Large or Extra Large Fonts still shows this problem.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007100205 Minefield/3.0a9pre ID:2007100205
verified fixed using  Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007100205 Minefield/3.0a9pre. Perhaps the larger font issue should be a spinoff bug?
Status: RESOLVED → VERIFIED
Haven't see any spin off bug yet. And after fighting some more with alignment issues in bug 411064, I've come to the conclusion that my concern from comment #7 was correct. Instead of using a fixed padding, we'd better not use this code path at all and fall back to the better working CSS solution:

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/widget/src/windows/nsNativeThemeWin.cpp&rev=3.116&mark=1517,1572-1575#1516

Use the following userChrome.css snippet to see the difference (unless you're using Aero):

.menu-text { -moz-appearance: none !important; }
This has regressed again!  Attaching screenshot.

~B
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Bryan: please file a new bug.
Status: REOPENED → RESOLVED
Closed: 13 years ago12 years ago
Resolution: --- → FIXED
(In reply to comment #18)
> Bryan: please file a new bug.

Gavin, I opened https://bugzilla.mozilla.org/show_bug.cgi?id=432357 but it was marked as a duplicate of https://bugzilla.mozilla.org/show_bug.cgi?id=332314.  A waste of time!  

~B
(In reply to comment #19)
> Gavin, I opened https://bugzilla.mozilla.org/show_bug.cgi?id=432357 but it was
> marked as a duplicate of https://bugzilla.mozilla.org/show_bug.cgi?id=332314. 
> A waste of time!

Sorry, hope it didn't take you too much time. On the bright side, Kai seems to have a good grasp on the problem, so perhaps he'll come up with a patch.
You need to log in before you can comment on or make changes to this bug.