Closed
Bug 1201318
Opened 8 years ago
Closed 8 years ago
[10.11] Some UI system fonts are wrong (Helvetica instead of San Francisco, makes menu items look wonky)
Categories
(Core :: Graphics: Text, defect)
Tracking
()
VERIFIED
FIXED
mozilla44
People
(Reporter: mstange, Assigned: jtd)
References
(Blocks 1 open bug)
Details
(Whiteboard: gfx-noted)
Attachments
(11 files, 2 obsolete files)
50.35 KB,
image/png
|
Details | |
33.51 KB,
image/png
|
Details | |
48.99 KB,
image/png
|
Details | |
40 bytes,
text/x-review-board-request
|
jtd
:
review+
lizzard
:
approval-mozilla-beta+
|
Details |
40 bytes,
text/x-review-board-request
|
jtd
:
review+
lizzard
:
approval-mozilla-beta+
|
Details |
38.35 KB,
patch
|
mstange
:
review+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
228.85 KB,
image/png
|
Details | |
62.37 KB,
image/png
|
Details | |
50.95 KB,
image/png
|
Details | |
93.37 KB,
image/png
|
Details | |
31.20 KB,
patch
|
mstange
:
review+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
In the recent 10.11 Betas, many pieces of UI text in Firefox have been using the wrong font.
I looked into what happens: [NSFont menuBarFontOfSize:0.0] returns a font with familyName .AppleSystemUIFont, which is not in gfxMacPlatformFontList::mSystemFontFamilies because CTFontManagerCopyAvailableFontFamilyNames() doesn't list it. Once that has been worked around, the next issue is that MacOSFontEntry::GetFontRef() returns null because CGFontCreateWithFontName(".AppleSystemUIFont") returns null. This too can be worked around by using CTFontCopyGraphicsFont((CTFontRef)[NSFont fontWithName:psname size:0], nullptr).
I've filed the following bug with Apple about this:
rdar://22548869/
> Summary:
> NSFont has a few APIs to get system fonts, for example
> +[NSFont systemFontOfSize:] and +[NSFont menuBarFontOfSize:]. On 10.11, some of
> these APIs return fonts with familyName .SFNSText, and others return fonts with
> familyName .AppleSystemUIFont.
>
> .SFNSText is working correctly.
> .AppleSystemUIFont however is working differently - it neither shows up in the
> list returned by CTFontManagerCopyAvailableFontFamilyNames(), nor is it
> accepted by CGFontCreateWithFontName. Both of the latter factors break system
> fonts in the Firefox UI.
>
> Steps to Reproduce:
> NSFont* font = [NSFont menuBarFontOfSize:0.0];
> for (NSArray* face in [[NSFontManager sharedFontManager] availableMembersOfFontFamily:[font familyName]]) {
> CGFontRef cgFont = CGFontCreateWithFontName((CFStringRef)[face objectAtIndex:0]);
> assert(cgFont);
> }
>
> Expected Results:
> Either .AppleSystemUIFont should be supported by both CGFontCreateWithFontName
> and CTFontManagerCopyAvailableFontFamilyNames, or all NSFont APIs should return
> fonts with familyName .SFNSText, or the functionality change should be
> documented as intentional so that we can work around it in Firefox.
>
> Actual Results:
> CGFontCreateWithFontName returns null for .AppleSystemUIFont, and
> CTFontManagerCopyAvailableFontFamilyNames doesn't list it.
>
> Version:
> 10.11 Beta 15A279b
Reporter | ||
Comment 1•8 years ago
|
||
Reporter | ||
Comment 2•8 years ago
|
||
Reporter | ||
Comment 3•8 years ago
|
||
Assignee | ||
Comment 4•8 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #0) > I looked into what happens: [NSFont menuBarFontOfSize:0.0] returns a font > with familyName .AppleSystemUIFont, which is not in > gfxMacPlatformFontList::mSystemFontFamilies because > CTFontManagerCopyAvailableFontFamilyNames() doesn't list it. Once that has > been worked around, the next issue is that MacOSFontEntry::GetFontRef() > returns null because CGFontCreateWithFontName(".AppleSystemUIFont") returns > null. This too can be worked around by using > CTFontCopyGraphicsFont((CTFontRef)[NSFont fontWithName:psname size:0], > nullptr). Hmmm, that's obnoxious. The whole notion of "hidden" UI fonts seems sort of ill-conceived by Apple.
Reporter | ||
Comment 5•8 years ago
|
||
Should we land a workaround now? We can still remove it if Apple changes things.
Reporter | ||
Comment 6•8 years ago
|
||
Bug 1201318 - Use nsAutoReleasePool from nsCocoaUtils.h. r?jtd The next patch will use nsCocoaUtils::GetStringForNSString, so I need to add the include to nsCocoaUtils.h anyway.
Attachment #8656366 -
Flags: review?(jdaggett)
Reporter | ||
Comment 7•8 years ago
|
||
Bug 1201318 - Factor out AddFamily. r?jtd
Attachment #8656367 -
Flags: review?(jdaggett)
Reporter | ||
Comment 8•8 years ago
|
||
Bug 1201318 - Add font families for system fonts in case those aren't listed in CTFontManagerCopyAvailableFontFamilyNames(). r?jtd
Attachment #8656368 -
Flags: review?(jdaggett)
Reporter | ||
Comment 9•8 years ago
|
||
Bug 1201318 - Fall back to getting the CGFontRef from NSFont to workaround 10.11 weirdness. r?jtd
Attachment #8656369 -
Flags: review?(jdaggett)
Updated•8 years ago
|
Whiteboard: gfx-noted
Reporter | ||
Comment 10•8 years ago
|
||
Comment on attachment 8656367 [details] MozReview Request: Bug 1201318 - Factor out AddFamily. r?jtd Bug 1201318 - Factor out AddFamily. r?jtd
Reporter | ||
Comment 11•8 years ago
|
||
Comment on attachment 8656368 [details] MozReview Request: Bug 1201318 - Add font families for system fonts in case those aren't listed in CTFontManagerCopyAvailableFontFamilyNames(). r?jtd Bug 1201318 - Add font families for system fonts in case those aren't listed in CTFontManagerCopyAvailableFontFamilyNames(). r?jtd
Reporter | ||
Comment 12•8 years ago
|
||
Comment on attachment 8656369 [details] MozReview Request: Bug 1201318 - Fall back to getting the CGFontRef from NSFont to work around 10.11 weirdness. r?jtd Bug 1201318 - Fall back to getting the CGFontRef from NSFont to work around 10.11 weirdness. r?jtd
Attachment #8656369 -
Attachment description: MozReview Request: Bug 1201318 - Fall back to getting the CGFontRef from NSFont to workaround 10.11 weirdness. r?jtd → MozReview Request: Bug 1201318 - Fall back to getting the CGFontRef from NSFont to work around 10.11 weirdness. r?jtd
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → mstange
Assignee | ||
Comment 13•8 years ago
|
||
Chromium has fixed a similar, related problem: https://code.google.com/p/chromium/issues/detail?id=521034
Assignee | ||
Comment 14•8 years ago
|
||
Safari on 10.11 uses the magical Apple system font keywords rather than real font names: http://people.mozilla.org/~jdaggett/tests/systemfontlist.html I suspect that 'AppleSystemUIFont' is somehow an alias that changes based on the size of the font from xxx Text to xxx Display depending upon the size. I think we need to investigate what happens across sizes for these families...
Reporter | ||
Comment 15•8 years ago
|
||
I'm not really sure what to look for. Do you want to take over this bug? I have a (possibly unrelated) question about changing font characteristics based on size. On HiDPI, I expect regular Cocoa apps to draw text with the size in logical screen units under a 2x scale transform, and intuitively I'd say that Gecko probably draws it with the size in physical screen units without a scale transform. Will that make a difference in rendering?
Assignee | ||
Comment 16•8 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #15) > I'm not really sure what to look for. Do you want to take over this bug? > > I have a (possibly unrelated) question about changing font characteristics > based on size. On HiDPI, I expect regular Cocoa apps to draw text with the > size in logical screen units under a 2x scale transform, and intuitively I'd > say that Gecko probably draws it with the size in physical screen units > without a scale transform. Will that make a difference in rendering? Yeah, I think I need to review the WWDC video (2013?) about "Dynamic Text" and maybe ping some people at Apple. It seems to me that the 'AppleSystemUIFont' may be what they're using and when a specific NSFont object is instantiated with a size, the choice of which underlying font to use is made at that point (e.g. .SFNSText or .SFNSDisplay). This is "optical sizing" and there are various, incompatible schemes for this across OS and fonts. Adobe has one (the 'size' OpenType feature which really shouldn't have been a feature) and so does Microsoft (the usLowerOpticalPointSize and usUpperOpticalPointSize additions to the OS/2 table). Any idea about the priority of this? Seems like it should be relatively high but I'd just like to confirm that.
Reporter | ||
Comment 17•8 years ago
|
||
(In reply to John Daggett (:jtd) from comment #16) > Any idea about the priority of this? Seems like it should be relatively high > but I'd just like to confirm that. I think it would be very embarrassing to ship with broken context menu and URL bar fonts when 10.11 is released, so yes, I think this needs to be a high priority. Some websites estimate the ship date of 10.11 to be at the end of September, so we need to be ready very soon.
Assignee | ||
Updated•8 years ago
|
Assignee: mstange → jdaggett
Assignee | ||
Comment 18•8 years ago
|
||
Markus, I'm going to take this one if you don't mind. As I suspected, it looks like this is much more complicated than simply figuring out how to look up private system fonts. The use of optical sizing means there are new API's that we should be using for system fonts. The relevant presentation on 10.11 and the use of optical sizing: https://developer.apple.com/videos/wwdc/2015/?id=804 If you look through the PDF, there's a section entitled "Font API Pitfalls" (p. 295) which includes points like: - Don’t Access Fonts with Private Names - Don’t Access System Font by Name - Don’t Draw System Font at a Different-Than-Nominal Point Size The first two imply one should simply lookup and use the system font and never touch the family name. The last one means we shouldn't take a given system font and use it at a different size, the size needs to be part of the system font lookup, since the optical size rules will choose a different underlying font based on the size. I think the solution here is to probably tweak the handling of UI fonts such that we pass a special system fontname (e.g. -apple-system-xxx?) down to the font system and have a special subclass of gfxMacFontFamily that deals with the details of the actual lookup. I haven't completely thought this through but it's clear from the WWDC slides that relying on family name lookups isn't going to work. The end result needs to be that any time a new gfxFont instance is created for a system font family, some form of systemFontOfSize: needs to get called to assure that the right font gets picked up. Note that I think this will also clean up some of the nastiness associated with the iOS code on bug 1170986.
Assignee | ||
Comment 19•8 years ago
|
||
Comment on attachment 8656366 [details] MozReview Request: Bug 1201318 - Use nsAutoReleasePool from nsCocoaUtils.h. r?jtd Clearing out the review requests for now, as I think we need to investigate this a little more and do a different approach.
Attachment #8656366 -
Flags: review?(jdaggett)
Assignee | ||
Updated•8 years ago
|
Attachment #8656367 -
Flags: review?(jdaggett)
Assignee | ||
Updated•8 years ago
|
Attachment #8656368 -
Flags: review?(jdaggett)
Assignee | ||
Updated•8 years ago
|
Attachment #8656369 -
Flags: review?(jdaggett)
Comment 20•8 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #17) > (In reply to John Daggett (:jtd) from comment #16) > > Any idea about the priority of this? Seems like it should be relatively high > > but I'd just like to confirm that. > > I think it would be very embarrassing to ship with broken context menu and > URL bar fonts when 10.11 is released, so yes, I think this needs to be a > high priority. Some websites estimate the ship date of 10.11 to be at the > end of September, so we need to be ready very soon. Yup, Sep 30. About as official as it can get: https://www.apple.com/osx/elcapitan-preview/
Reporter | ||
Comment 21•8 years ago
|
||
Firefox 41, which will be released on September 22nd, is currently in the RC stage, so any fix for this bug won't be accepted into it. That means that this bug won't be fixed by the time 10.11 is released. Firefox 42 will be released on November 3rd.
Assignee | ||
Comment 22•8 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #21) > Firefox 41, which will be released on September 22nd, is currently in the RC > stage, so any fix for this bug won't be accepted into it. That means that > this bug won't be fixed by the time 10.11 is released. > Firefox 42 will be released on November 3rd. I'd like to get this fixed this week. Then we can see what is required/possible in terms of pushing it out. But I think FF42 is probably the likely earliest release.
Reporter | ||
Comment 24•8 years ago
|
||
Here's Apple's response to the bug report I filed:
> This is the new reality in supporting the SF font. The system font is now a
> collection of virtual fonts that might not show up in user interface related
> list.
>
> You need to migrate to CoreText-based APIs such as CTFontDescriptor.
Comment 25•8 years ago
|
||
Does this mean we need to take some form of the patch in bug 1170986?
Assignee | ||
Comment 26•8 years ago
|
||
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #25) > Does this mean we need to take some form of the patch in bug 1170986? Not precisely but it does mean we need to treat system fonts like this differently. I'm going to rework the LookAndFeel code to pass down a "special" name for system fonts, something along the lines of what Safari is doing with names like "-apple-system". The font family name lookup code will then do the lookup using CoreText/NSFont magic. Note that this includes doing a size-specific lookup when creating a gfxFont object, as different sizes will map to different underlying fonts due to the way optical sizing works. In other words, sizes less than 20pt will map to a "text" flavor of San Francisco and sizes larger will map to a "display" font. So we need different underlying gfxFontEntry objects for each. I think the one outcome here is that the iOS code will actually become simpler with this distinction. Still a little fuzzy on what CTFontDescriptor magic is needed, but it may just be that this "virtual font" logic only works under 10.11. Webkit code isn't much of a help as they simply use a bunch of private API's (naturally...).
Reporter | ||
Comment 27•8 years ago
|
||
(In reply to John Daggett (:jtd) from comment #26) > I'm going to rework the LookAndFeel code to pass down a > "special" name for system fonts, something along the lines of what Safari is > doing with names like "-apple-system". The font family name lookup code will > then do the lookup using CoreText/NSFont magic. Why not make the LookAndFeel code pass down an NSFont pointer directly, instead of going through a magic string?
Assignee | ||
Comment 28•8 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #27) > Why not make the LookAndFeel code pass down an NSFont pointer directly, > instead of going through a magic string? That's a cross-platform API, the style system puts the name into the fontlist, which is in turn visible to authors. So better to do as Apple does and expose "-apple-system" as a magic font family name. In addition, the actual font under 10.11 will vary based on the size between .SF NS Text and .SF NS Display.
Assignee | ||
Updated•8 years ago
|
Attachment #8656366 -
Flags: review+
Assignee | ||
Updated•8 years ago
|
Attachment #8656367 -
Flags: review+
Assignee | ||
Updated•8 years ago
|
Attachment #8656368 -
Flags: review-
Assignee | ||
Updated•8 years ago
|
Attachment #8656369 -
Flags: review-
Reporter | ||
Comment 29•8 years ago
|
||
(In reply to John Daggett (:jtd) from comment #28) I thought Apple's magic font names were analogous to our platform font names like menu, icon, message-box etc. Shouldn't those be the names that we put into the font list?
Assignee | ||
Comment 30•8 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #29) > (In reply to John Daggett (:jtd) from comment #28) > > I thought Apple's magic font names were analogous to our platform font names > like menu, icon, message-box etc. Shouldn't those be the names that we put > into the font list? They use the *same* font across *all* of those calls, the only variation is the size and weight that come back. It simplifies things to make this assumption, since the lookup from -apple-system ==> font family is much quicker if there's only one or two families to deal with.
Reporter | ||
Comment 31•8 years ago
|
||
Oh, I see, thanks. I was getting confused by the size/weight-specific font names like -apple-system-headline.
Assignee | ||
Comment 32•8 years ago
|
||
Rework handling of system fonts under OSX. The LookAndFeel code now calls through to gfx code to lookup the system font family/style. It uses the same "meta" system font name that Webkit uses, -apple-system. The FindFamily method now takes a gfxFontStyle pointer so that the size can be used to look up the underlying font family name when running under 10.11. The code here assumes that (1) the system font family name is the same across UI font API calls and (2) that OSX 10.11 uses a combination of text/display font families for UI display. The handling of these families is hard-coded since there's no efficient API for determining this on the fly. But within debug code, both of these assumptions are confirmed so that we can flag when changes do occur in the future.
Attachment #8656368 -
Attachment is obsolete: true
Attachment #8656369 -
Attachment is obsolete: true
Attachment #8670760 -
Flags: review?(mstange)
Reporter | ||
Comment 33•8 years ago
|
||
Comment on attachment 8670760 [details] [diff] [review] patch, rework system font handling under OSX Review of attachment 8670760 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! ::: gfx/thebes/gfxMacPlatformFontList.mm @@ +647,5 @@ > sFontManager = [NSFontManager sharedFontManager]; > + > +#ifdef DEBUG > + // different system font API's always map to the same family under OSX, so > + // just assume that and omit a warning if that ever changes omit -> emit @@ +772,5 @@ > } > > +// System fonts under OSX may contain weird "meta" names but if we create > +// a new font using just the Postscript name, the NSFont api returns an object > +// with the actual real family name Can you add one example of the different names here? @@ +803,5 @@ > + nsAutoString familyName, key; > + nsCocoaUtils::GetStringForNSString(textFamilyName, familyName); > + GenerateFontListKey(familyName, key); > + gfxFontFamily* family = mSystemFontFamilies.GetWeak(key); > + NS_ASSERTION(family, "null system text font family"); So the "real family name" is one that is listed by CTFontManagerCopyAvailableFontFamilyNames()? @@ +819,5 @@ > + mSystemDisplayFontFamily = CheckFamily(family); > + } > + > +#if DEBUG > + // confirm that the optical size switch is at 20.0 Thank you for this. @@ +1081,5 @@ > +gfxFontFamily* > +gfxMacPlatformFontList::FindFamily(const nsAString& aFamily, gfxFontStyle* aStyle) > +{ > + nsAutoString key; > + GenerateFontListKey(aFamily, key); You're not using the variable "key" in this function.
Attachment #8670760 -
Flags: review?(mstange) → review+
Comment 34•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8a9bf6dbf9b3 https://hg.mozilla.org/integration/mozilla-inbound/rev/76f93328931b https://hg.mozilla.org/integration/mozilla-inbound/rev/39f705a5cb70
Comment 35•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8a9bf6dbf9b3 https://hg.mozilla.org/mozilla-central/rev/76f93328931b https://hg.mozilla.org/mozilla-central/rev/39f705a5cb70
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox44:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Reporter | ||
Comment 36•8 years ago
|
||
Comment on attachment 8670760 [details] [diff] [review] patch, rework system font handling under OSX Review of attachment 8670760 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/thebes/gfxMacPlatformFontList.mm @@ +653,5 @@ > + if ([sysFamily compare:[[NSFont boldSystemFontOfSize:0.0] familyName]] != NSOrderedSame || > + [sysFamily compare:[[NSFont controlContentFontOfSize:0.0] familyName]] != NSOrderedSame || > + [sysFamily compare:[[NSFont menuBarFontOfSize:0.0] familyName]] != NSOrderedSame || > + [sysFamily compare:[[NSFont toolTipsFontOfSize:0.0] familyName]] != NSOrderedSame) { > + NS_WARNING("system font types map to different font families -- please log a bug!!"); Hmm, I realized that I don't actually understand the purpose of this warning. Did you mean to use GetRealFamilyName here? Because this warning fires on my machine. systemFont: .SF NS Text boldSystemFont: .SF NS Text controlContentFont: .AppleSystemUIFont menuBarFont: .AppleSystemUIFont toolTipsFont: .AppleSystemUIFont
Reporter | ||
Updated•8 years ago
|
Flags: needinfo?(jdaggett)
Assignee | ||
Comment 37•8 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #36) > > + if ([sysFamily compare:[[NSFont boldSystemFontOfSize:0.0] familyName]] != NSOrderedSame || > > + [sysFamily compare:[[NSFont controlContentFontOfSize:0.0] familyName]] != NSOrderedSame || > > + [sysFamily compare:[[NSFont menuBarFontOfSize:0.0] familyName]] != NSOrderedSame || > > + [sysFamily compare:[[NSFont toolTipsFontOfSize:0.0] familyName]] != NSOrderedSame) { > > + NS_WARNING("system font types map to different font families -- please log a bug!!"); > > Hmm, I realized that I don't actually understand the purpose of this > warning. Did you mean to use GetRealFamilyName here? Because this warning > fires on my machine. > > systemFont: .SF NS Text > boldSystemFont: .SF NS Text > controlContentFont: .AppleSystemUIFont > menuBarFont: .AppleSystemUIFont > toolTipsFont: .AppleSystemUIFont Argh, yes, these are effectively the same font for size: 0.0 so we should use GetRealFamilyName here. I'll log a new bug to fix this up.
Flags: needinfo?(jdaggett)
Assignee | ||
Comment 38•8 years ago
|
||
Assignee | ||
Comment 39•8 years ago
|
||
Difference in similar menu item width is likely bug 1213280.
Assignee | ||
Comment 40•8 years ago
|
||
Here too, the width issue is likely bug 1213280.
Assignee | ||
Comment 41•8 years ago
|
||
Comment on attachment 8656366 [details] MozReview Request: Bug 1201318 - Use nsAutoReleasePool from nsCocoaUtils.h. r?jtd Approval Request Comment [Feature/regressing bug #]: latest OSX 10.11 doesn't use correct system font [User impact if declined]: incorrect font will display in UI elements [Describe test coverage new/current, TreeHerder]: stable on trunk [Risks and why]: low risk, simply selects correct system font [String/UUID change made/needed]: none
Attachment #8656366 -
Flags: approval-mozilla-beta?
Attachment #8656366 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 42•8 years ago
|
||
Comment on attachment 8656367 [details] MozReview Request: Bug 1201318 - Factor out AddFamily. r?jtd Approval Request Comment [Feature/regressing bug #]: latest OSX 10.11 doesn't use correct system font [User impact if declined]: incorrect font will display in UI elements [Describe test coverage new/current, TreeHerder]: stable on trunk [Risks and why]: low risk, simply selects correct system font [String/UUID change made/needed]: none
Attachment #8656367 -
Flags: approval-mozilla-beta?
Attachment #8656367 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 43•8 years ago
|
||
Comment on attachment 8670760 [details] [diff] [review] patch, rework system font handling under OSX Approval Request Comment [Feature/regressing bug #]: latest OSX 10.11 doesn't use correct system font [User impact if declined]: incorrect font will display in UI elements [Describe test coverage new/current, TreeHerder]: stable on trunk [Risks and why]: low risk, simply selects correct system font [String/UUID change made/needed]: none
Attachment #8670760 -
Flags: approval-mozilla-beta?
Attachment #8670760 -
Flags: approval-mozilla-aurora?
Comment 45•8 years ago
|
||
Comment on attachment 8670760 [details] [diff] [review] patch, rework system font handling under OSX landed before the merge.
Attachment #8670760 -
Flags: approval-mozilla-aurora?
Updated•8 years ago
|
Attachment #8656366 -
Flags: approval-mozilla-aurora?
Updated•8 years ago
|
Attachment #8656367 -
Flags: approval-mozilla-aurora?
Comment 47•8 years ago
|
||
The font in the address bar & context menu is getting worse. It's harder to read than ever. I'm using Firefox 42.0 on OS X El Capitan 10.11.1
Comment 48•8 years ago
|
||
(In reply to blaguskida15 from comment #47) > Created attachment 8683713 [details] > Address Bar & Context Menu Font Issue > > The font in the address bar & context menu is getting worse. It's harder to > read than ever. > > I'm using Firefox 42.0 on OS X El Capitan 10.11.1 As mentioned in bug 1218421 this has only been fixed in Firefox 44+. I doubt we'll fix this in Firefox 42 but we might be able to uplift this to Firefox 43 (currently in Beta). John, what are the chances we can uplift this?
Comment 49•8 years ago
|
||
Are there any temporary workarounds I can use in the meantime?
Assignee | ||
Comment 50•8 years ago
|
||
(In reply to blaguskida15 from comment #49) > Are there any temporary workarounds I can use in the meantime? Sorry, no workaround.
Assignee | ||
Comment 51•8 years ago
|
||
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #48) > John, what are the chances we can uplift this? I've already nominated it for approval-beta! Sylvestre, approval-beta ping. This is actually pretty important as it will not only fix our UI fonts in 10.11 but it will also allow authors to actually *use* the system font. Because of the way it strangely uses hidden font names, this is currently difficult to do. The folks at Medium have been blogging about this and I've seen a draft where they explicitly say "can't do this with Firefox", which is something I'd like to be able to push back on.
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(sledru)
Comment 52•8 years ago
|
||
Redirecting to Liz as she is the owner of 43.
Flags: needinfo?(sledru) → needinfo?(lhenry)
Comment on attachment 8670760 [details] [diff] [review] patch, rework system font handling under OSX Let's uplift this to beta, we want the font to look good.
Flags: needinfo?(lhenry)
Attachment #8670760 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8656367 [details] MozReview Request: Bug 1201318 - Factor out AddFamily. r?jtd Please uplift to beta.
Attachment #8656367 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8656366 [details] MozReview Request: Bug 1201318 - Use nsAutoReleasePool from nsCocoaUtils.h. r?jtd Please uplift to beta.
Attachment #8656366 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 56•8 years ago
|
||
the 3rd patch does not apply clean to beta: grafting 307340:39f705a5cb70 "Bug 1201318 - revise OSX system font handling. r=mstange" merging gfx/thebes/gfxDWriteFontList.cpp merging gfx/thebes/gfxDWriteFontList.h merging gfx/thebes/gfxFcPlatformFontList.cpp merging gfx/thebes/gfxFcPlatformFontList.h merging gfx/thebes/gfxFontEntry.cpp merging gfx/thebes/gfxGDIFontList.cpp merging gfx/thebes/gfxMacPlatformFontList.mm merging gfx/thebes/gfxPlatformFontList.cpp warning: conflicts during merge. merging gfx/thebes/gfxPlatformFontList.cpp incomplete! (edit conflicts, then use 'hg resolve --mark') merging gfx/thebes/gfxPlatformFontList.h merging gfx/thebes/gfxPlatformMac.cpp merging gfx/thebes/gfxTextRun.cpp warning: conflicts during merge. merging gfx/thebes/gfxTextRun.cpp incomplete! (edit conflicts, then use 'hg resolve --mark') abort: unresolved conflicts, can't continue (use hg resolve and hg graft --continue) can you take a look, thanks!
Flags: needinfo?(jdaggett)
Assignee | ||
Comment 57•8 years ago
|
||
(In reply to Carsten Book [:Tomcat] from comment #56) > the 3rd patch does not apply clean to beta: > > grafting 307340:39f705a5cb70 "Bug 1201318 - revise OSX system font handling. > r=mstange" > merging gfx/thebes/gfxDWriteFontList.cpp > merging gfx/thebes/gfxDWriteFontList.h > merging gfx/thebes/gfxFcPlatformFontList.cpp > merging gfx/thebes/gfxFcPlatformFontList.h > merging gfx/thebes/gfxFontEntry.cpp > merging gfx/thebes/gfxGDIFontList.cpp > merging gfx/thebes/gfxMacPlatformFontList.mm > merging gfx/thebes/gfxPlatformFontList.cpp > warning: conflicts during merge. > merging gfx/thebes/gfxPlatformFontList.cpp incomplete! (edit conflicts, then > use 'hg resolve --mark') > merging gfx/thebes/gfxPlatformFontList.h > merging gfx/thebes/gfxPlatformMac.cpp > merging gfx/thebes/gfxTextRun.cpp > warning: conflicts during merge. > merging gfx/thebes/gfxTextRun.cpp incomplete! (edit conflicts, then use 'hg > resolve --mark') > abort: unresolved conflicts, can't continue > (use hg resolve and hg graft --continue) > > can you take a look, thanks! Yeah, the patch was done on top of changes for bug 1182361. I'm going to put together a tweaked patch for the beta branch.
Flags: needinfo?(jdaggett)
Tracking for 43 and 44, mostly to make sure we land this in 43.
Assignee | ||
Comment 60•8 years ago
|
||
The original patch landed for this was done on top of a refactoring of the pref font handling code (bug 1182361). I'd rather not uplift the patches for that bug, so I've limited the changes here to a OSX-specific. Instead of modifying the parameters to the FindFamily method shared across platforms, this version creates a new, #ifdef OS_MACOSX version that uses a method, FindFamilyWithStyle, to achieve the same behavior. Other parts of the patch are unchanged. Approval Request Comment [Feature/regressing bug #]: system font is not correct under OSX 10.11 [User impact if declined]: menus and UI will appear slightly off [Describe test coverage new/current, TreeHerder]: landed over a month ago on trunk, no problems report [Risks and why]: reduced version of the patch landed on trunk/aurora [String/UUID change made/needed]: none
Attachment #8687783 -
Flags: review?(mstange)
Assignee | ||
Comment 61•8 years ago
|
||
Comment on attachment 8687783 [details] [diff] [review] patch, rework system font handling under OSX (for beta) Approval Request Comment - see last comment
Attachment #8687783 -
Flags: approval-mozilla-beta?
Reporter | ||
Comment 62•8 years ago
|
||
Comment on attachment 8687783 [details] [diff] [review] patch, rework system font handling under OSX (for beta) Review of attachment 8687783 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/thebes/gfxMacPlatformFontList.mm @@ +653,5 @@ > + if ([sysFamily compare:[[NSFont boldSystemFontOfSize:0.0] familyName]] != NSOrderedSame || > + [sysFamily compare:[[NSFont controlContentFontOfSize:0.0] familyName]] != NSOrderedSame || > + [sysFamily compare:[[NSFont menuBarFontOfSize:0.0] familyName]] != NSOrderedSame || > + [sysFamily compare:[[NSFont toolTipsFontOfSize:0.0] familyName]] != NSOrderedSame) { > + NS_WARNING("system font types map to different font families -- please log a bug!!"); So you don't want to integrate a fix for bug 1214490 into this patch?
Attachment #8687783 -
Flags: review?(mstange) → review+
Assignee | ||
Comment 63•8 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #62) > So you don't want to integrate a fix for bug 1214490 into this patch? I'm going to upload patches for this one along with bug 1213280 soonish, hopefully tomorrow.
Should this uplift wait to make sure that it lands alongside your fix from bug 1213280?
Flags: needinfo?(jdaggett)
Assignee | ||
Comment 65•8 years ago
|
||
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #64) > Should this uplift wait to make sure that it lands alongside your fix from > bug 1213280? It doesn't need to wait, this is the more crucial fix since without this we're choosing Helvetica Neue for UI instead of San Francisco. The fix for 1213280 will improve the spacing of text in San Francisco.
Flags: needinfo?(jdaggett)
Comment on attachment 8687783 [details] [diff] [review] patch, rework system font handling under OSX (for beta) Rebased for beta, system font fix.
Attachment #8687783 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 67•8 years ago
|
||
seems this has problems to apply to beta: patching file gfx/thebes/gfxMacPlatformFontList.h Hunk #2 FAILED at 73 1 out of 3 hunks FAILED -- saving rejects to file gfx/thebes/gfxMacPlatformFontList.h.rej patching file gfx/thebes/gfxMacPlatformFontList.mm Hunk #3 succeeded at 740 with fuzz 2 (offset 23 lines). patch failed, unable to continue (try -v) patch failed, rejects left in working directory errors during apply, please fix and refresh rework-mac-system-font-lookups-v3-beta.patch John, could you take a look, thanks!
Flags: needinfo?(jdaggett)
Assignee | ||
Comment 68•8 years ago
|
||
Huh, wonder why that happened. The beta patch applies just fine on top of the other two patches for me. Anywho, I went ahead and pushed these to beta: https://hg.mozilla.org/releases/mozilla-beta/rev/5284eeaeb23e https://hg.mozilla.org/releases/mozilla-beta/rev/b6f944763659 https://hg.mozilla.org/releases/mozilla-beta/rev/01ac7ddda781 Pinged lizzard and kwierso on IRC to let them know before landing.
Flags: needinfo?(jdaggett)
Assignee | ||
Updated•8 years ago
|
Updated•8 years ago
|
Flags: qe-verify+
Comment 69•8 years ago
|
||
Managed to reproduce this issue on Mac OS X 10.11.1 with the Firefox 42.0b1 (20150921151815) build. The issue is no longer reproducible on the Mac OS X 10.11.1, using Firefox 43.0b9 (20151203163240) and FIrefox 44.0a2 (2015-12-08).
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Comment 71•7 years ago
|
||
Is it correct that this bug implements "font-family: -apple-system" for web content in Firefox 43 on MAC OS X?
You need to log in
before you can comment on or make changes to this bug.
Description
•