Open Bug 435825 Opened 16 years ago Updated 2 years ago

Rework the metric computations for uxthemed menus and eliminate the use of magic numbers

Categories

(Core :: Widget: Win32, defect, P3)

x86
Windows Vista
defect

Tracking

()

People

(Reporter: kliu, Unassigned)

References

()

Details

Attachments

(1 file, 3 obsolete files)

Attached patch patch v1 (obsolete) — Splinter Review
The current uxtheme menu code in nsNativeThemeWin.cpp contains a lot of magic numbers in GetWidgetPadding, and the methods used to compute the metrics are at odds with the documentation that Microsoft provides (well, if you count their sample code as documentation; see URL).

This patch refactors the methods used to compute the various menu metrics so that they are consistent with Microsoft's documented approach, and along the way, this allows us to get rid of the hard-coded values used for the Vista menu.  This also fixes three minor problems with the current uxtheme menu appearance:

1) At large font sizes (where the height of the font exceeds the size of the checkbox height), the existing code renders the menu items a couple of px shorter than native.

2) Icons should be horizontally centered with respect to the radio/checkbox graphics.

3) Menu separators are missing a couple of pixels of vertical padding.

I've tested this patch with various font sizes, and everything works as expected.

(I expect bug 435604 to land before this, so this patch was made against that patch instead of trunk.)
Attachment #322597 - Flags: superreview?(vladimir)
Attachment #322597 - Flags: review?(vladimir)
Blocks: 427392
Attached patch patch v2 (obsolete) — Splinter Review
Notable changes in this patch:

* This patch gets rid of the hard-coded values for NS_THEME_MENUARROW (I didn't notice them when I made patch v1)

* Added comment to menu.css

* This patch was made against the patches in bug 435604 and bug 403458.
Attachment #322597 - Attachment is obsolete: true
Attachment #323251 - Flags: superreview?(vladimir)
Attachment #323251 - Flags: review?(vladimir)
Attachment #322597 - Flags: superreview?(vladimir)
Attachment #322597 - Flags: review?(vladimir)
Blocks: 436779
Flags: wanted1.9.1?
No longer depends on: 435604
Attached patch patch v2 (unbitrot) (obsolete) — Splinter Review
Patch applied on top of bug 441452 and bug 403458.
Attachment #323251 - Attachment is obsolete: true
Attachment #326689 - Flags: superreview?(vladimir)
Attachment #326689 - Flags: review?(vladimir)
Attachment #323251 - Flags: superreview?(vladimir)
Attachment #323251 - Flags: review?(vladimir)
Attachment #326689 - Attachment is obsolete: true
Attachment #326689 - Flags: superreview?(vladimir)
Attachment #326689 - Flags: review?(vladimir)
Product: Core → SeaMonkey
Component: Themes → Widget: Win32
Product: SeaMonkey → Core
Flags: wanted1.9.1? → wanted1.9.1+
Priority: -- → P3
I unbitrotted this patch, but I haven't tested how this affects XP and classic
I also didn't test how this affects Vista. Under the bookmarks menu, there are three major issues that I noticed: the menu arrows are not lined up, the icons in the gutter are no longer centered, and the "Mozilla Firefox" bookmark folder is too tall.
Yea, there's a bug I introduced when I did the menu arrow metrics; that wasn't present in the previous versions of the patch.  I've been meaning to get around to unbitrotting this and correcting that mistake, but I've been busy (and traveling).  I may have a chance at revisiting this later tonight.

This should have no effect on XP and Classic, as the menus there are rendered with the "classic" code path with the only difference between those two being the appearance of the beveling.  I had also tested this patch with third-party menu styles (via Windows Blinds), and it looks fine there...
QA Contact: themes → win32

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.

Assignee: kliu → nobody
Status: ASSIGNED → NEW
Severity: normal → S4
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: