925241 - Remove unnecessary Quickdraw font name lookup code

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Josh Aas]

Attachment: Fix v1.0

Please review my patch carefully as I do not know much about this code. Resolves the following warnings:

gfx/thebes/gfxMacPlatformFontList.mm:787:36 [-Wdeprecated-declarations] 'ATSFontFamilyFindFromQuickDrawName' is deprecated: Use CTFontCreateWithQuickdrawInstance()
gfx/thebes/gfxMacPlatformFontList.mm:794:25 [-Wdeprecated-declarations] 'ATSFontFamilyGetName' is deprecated: Use CTFontCopyFamilyName()

Comment 1

[Jonathan Kew [:jfkthame]]

Comment on attachment 815245 [details] [diff] [review]
Fix v1.0

Review of attachment 815245 [details] [diff] [review]:
-----------------------------------------------------------------

This looks correct to me, though I don't think I have any fonts installed with which I can actually test it. I'm tempted to suggest we could simply drop this code; the relatively obscure cases that it was originally aimed at solving have probably faded into irrelevance by now. (It doesn't seem to apply to the Futura family that Apple is currently shipping, for example, despite the code comment.)

jdaggett, WDYT? Should we take this patch (which looks fine), or try just ripping the code out altogether?

Comment 2

[Josh Aas]

Dropping the code altogether would be even better!

Comment 3

[Josh Aas]

Attachment: Remove Code v1.0

Comment 4

[John Daggett (:jtd)]

The prefs code used to use QD name lookups.  QD created fake families for faces that weren't the standard regular/bold/italic/bold italic faces.  So the condensed faces of Futura would end up in a synthetic family called "Futura Condensed".  At this point I don't think we need to worry about such a scenario.

Steps to reproduce the "problem" scenario after the code is removed:

1. Pre-Gecko 1.8, create a new profile
2. Open Prefs > Content, change font to "Futura Condensed".
3. Close pre-gecko 1.8
4. Open trunk build including code removal

Result: text pref will show up blank

Comment 5

[Jonathan Kew [:jfkthame]]

(In reply to John Daggett (:jtd) from comment #4)
> The prefs code used to use QD name lookups.  QD created fake families for
> faces that weren't the standard regular/bold/italic/bold italic faces.  So
> the condensed faces of Futura would end up in a synthetic family called
> "Futura Condensed".  At this point I don't think we need to worry about such
> a scenario.

Yeah, that was my feeling.

> 
> Steps to reproduce the "problem" scenario after the code is removed:
> 
> 1. Pre-Gecko 1.8, create a new profile
> 2. Open Prefs > Content, change font to "Futura Condensed".
> 3. Close pre-gecko 1.8
> 4. Open trunk build including code removal
> 
> Result: text pref will show up blank

AFAICT, this will already fail with the current (ATS-based) trunk code; at least on my 10.7 system, it does -not- successfully resolve "Futura Condensed" to the Futura family.

(I haven't spent a lot of time on this, though; maybe I'm just not testing correctly.)

Comment 6

[Josh Aas]

http://hg.mozilla.org/integration/mozilla-inbound/rev/4fd346e0e404

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4fd346e0e404