Closed Bug 335683 Opened 15 years ago Closed 15 years ago

Use kThemeMenuItemFont for -moz-pull-down-menu

Categories

(Core Graveyard :: GFX: Mac, enhancement)

PowerPC
macOS
enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: stefanh, Assigned: stefanh)

References

Details

(Keywords: fixed1.8.1)

Attachments

(2 files)

Atm, there is no way for a theme author to set the correct font (size) for menus. Using "font: menu;" will produce the standard system font (kThemeSystemFont) - 1 px smaller than the font used in mac os x menus.

Unfortunately, "font: menu;" is used everywhere in the themes - even for buttons etc. So, just switching font id will break a lot of stuff.

The css 3 working draft at http://www.w3.org/TR/2002/WD-css3-ui-20020802/#system0 describes the fonts "menu" and  "pull-down-menu" like this:
------------------------------------------------------------------
menu
    The font used in menus (e.g., dropdown menus and menu lists). 
pull-down-menu
    The specific font used in pull-down menus.
------------------------------------------------------------------

So, if I haven't misunderstood anything, "-moz-pull-down-menu" looks like a candidate for the "real" menu font.
Attached file Testcase
Testcase, showing difference  between "font: menu;" and "font: -moz-pull-down-menu;"
This patch will make use of kThemeMenuItemFont for "-moz-pull-down-menu". I patched the nsDeviceContextMac.cpp that is in widget/src/mac as well (not sure who uses that).
Assignee: joshmoz → stefanh
Status: NEW → ASSIGNED
Attachment #220011 - Flags: review?(mark)
Blocks: 308010
Comment on attachment 220011 [details] [diff] [review]
Proposed patch - use kThemeMenuItemFont

I now see that the css 3 system fonts is at risk being removed from the spec (http://www.w3.org/TR/css3-ui/#atrisk).

We could of course also use a new name like "-moz-mac-menuitem" if that's preferable.
The patch looks OK.  I'm not the right guy to decide if the name should be reused or a new one should be picked.

I think the extra copy of nsDeviceContextMac.cpp happened when some things were moved out of gfx into widget.  The widget copy isn't used by anyone as far as I know.  Ccing Stuart because I think he was the one behind that big move.
Comment on attachment 220011 [details] [diff] [review]
Proposed patch - use kThemeMenuItemFont

smfr, this patch makes use of kThemeMenuItemFont for -moz-pull-down-menu. One question is if we should use a new css keyword instead. Thoughts?
Attachment #220011 - Flags: superreview?(sfraser_bugs)
Attachment #220011 - Flags: review?(mark) → review+
Comment on attachment 220011 [details] [diff] [review]
Proposed patch - use kThemeMenuItemFont

Trying dbaron since the main question is re-using the existing "moz-pull-down-menu" or not.
Attachment #220011 - Flags: superreview?(sfraser_bugs) → superreview?(dbaron)
Comment on attachment 220011 [details] [diff] [review]
Proposed patch - use kThemeMenuItemFont

Trying roc, since dbaron seems busy.
Attachment #220011 - Flags: superreview?(dbaron) → superreview?(roc)
Comment on attachment 220011 [details] [diff] [review]
Proposed patch - use kThemeMenuItemFont

I don't see why not.
Attachment #220011 - Flags: superreview?(roc) → superreview+
Comment on attachment 220011 [details] [diff] [review]
Proposed patch - use kThemeMenuItemFont

This would make it possible for theme authors to use the correct font in our xul menus.
Attachment #220011 - Flags: approval-branch-1.8.1?(mark)
Attachment #220011 - Flags: approval-branch-1.8.1?(mark) → approval-branch-1.8.1+
Fix checked in on trunk & branch.

Seems like nsDeviceContextMac.cpp exists on trunk but not on 1.8 branch. According to the makefile, that file is not built on trunk. It seems to be unused. So we should probably remove it?
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Blocks: 342515
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.