Closed
Bug 335683
Opened 19 years ago
Closed 19 years ago
Use kThemeMenuItemFont for -moz-pull-down-menu
Categories
(Core Graveyard :: GFX: Mac, enhancement)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: stefanh, Assigned: stefanh)
References
Details
(Keywords: fixed1.8.1)
Attachments
(2 files)
312 bytes,
application/xhtml+xml
|
Details | |
5.04 KB,
patch
|
mark
:
review+
roc
:
superreview+
mark
:
approval-branch-1.8.1+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•19 years ago
|
||
Testcase, showing difference between "font: menu;" and "font: -moz-pull-down-menu;"
Assignee | ||
Comment 2•19 years ago
|
||
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 | ||
Comment 3•19 years ago
|
||
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.
Comment 4•19 years ago
|
||
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.
Assignee | ||
Comment 5•19 years ago
|
||
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)
Updated•19 years ago
|
Attachment #220011 -
Flags: review?(mark) → review+
Assignee | ||
Comment 6•19 years ago
|
||
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)
Assignee | ||
Comment 7•19 years ago
|
||
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+
Assignee | ||
Comment 9•19 years ago
|
||
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)
Updated•19 years ago
|
Attachment #220011 -
Flags: approval-branch-1.8.1?(mark) → approval-branch-1.8.1+
Comment 10•19 years ago
|
||
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?
Updated•16 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•