[OSX] Inspector font names are not always recognized

VERIFIED FIXED

Status

()

VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: petruta.rasa, Assigned: jfkthame)

Tracking

({regression})

Trunk
All
Mac OS X
regression
Points:
---

Firefox Tracking Flags

(firefox48 wontfix, firefox49+ wontfix, firefox50 verified, firefox51 verified)

Details

(URL)

Attachments

(2 attachments)

Created attachment 8780121 [details]
missing_font_names.png

[Notes]:
- This is not reproducible on Win 10 64-bit and Ubuntu 16.04 64-bit.

[Affected versions]:
- Firefox 49 beta 2
- latest Aurora 50.0a2 2016-08-11
- latest Nightly 51.0a1 2016-08-11

[Affected platforms]:
- Mac OS X 10.9.5
- Mac OS X 10.12 beta

[Steps to reproduce]:
1. Open a website with heavy content(eg https://goo.gl/GS0h5t)
2. Open the Developer Tools - Inspector and go to Fonts tab
3. Check the fonts names

[Expected result]:
- All the fonts are displayed

[Actual result]:
- Some of the remote font names are shown as "(MISSING NAME)"

[Regression range]:
- This a Firefox 44 regression:
- moz-central: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2387ada864282880d3a498d643abe3d8b887ee59&tochange=e193b4da0a8c1025aa76a403c64663ff1cd41709

- moz-inbound: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=15104d2224f4d1c794994941e880d616cf9aa486&tochange=5baceaa01feee58e5dee795f9ef3bd2a596c8448

According to m-i, this was caused by bug 1211867, bug 1193050 or bug 1211141. Adding Jonathan and Frederic to CC.
QA Whiteboard: [qe-dthtml]

Updated

2 years ago
Whiteboard: [devtools-html][triage]
(Assignee)

Comment 1

2 years ago
I guess this must be a result of bug 1193050 changing some details of how the 'name' table gets processed. Depending what is going on, it might need to be reported upstream as an OTS issue; but first we should understand exactly what changed.
Blocks: 1193050
(Assignee)

Comment 2

2 years ago
OK, I see what's going on; it's a Gecko bug, not OTS.

Note that prior to bug 1193050, Firefox on OS X showed "fake" names ("OTS derived font") for those faces. This is something that OTS was inserting where the font lacked Mac-platform name table entries. It no longer does this if the font contains valid Windows-platform names -- which I think is OK. So the decoded font resource ends up having _only_ Windows-platform names, whereas previously it also contained (fake) Mac-platform names.

Now, when we get a font with only Windows names, we have code that was intended to use those as a fallback (on the Mac platform; we'd already use them by default on other platforms). However, it turns out that gfxFontUtils::ReadNames has a bug in how it handles the platform ID, such that this fallback-to-Windows-names-on-Mac never actually works. And so we end up with the "(MISSING NAME)" string.

By fixing that code, we'll not only get rid of the "(MISSING NAME)" junk, we'll actually show the correct face names for these resources, instead of the generic "OTS derived font" that we used to get.
(Assignee)

Comment 3

2 years ago
Created attachment 8780157 [details] [diff] [review]
Fix handling of platform ID in gfxFontUtils::ReadNames, so that fallback to Windows-platform names on Mac works as intended
Attachment #8780157 - Flags: review?(jmuizelaar)
(Assignee)

Updated

2 years ago
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Comment on attachment 8780157 [details] [diff] [review]
Fix handling of platform ID in gfxFontUtils::ReadNames, so that fallback to Windows-platform names on Mac works as intended

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

This would've been easier to review with cosmetics separated out into a separate patch. It was a bit of a struggle to find out what was actually changing. It's probably worth making the commit message a bit more specific about what's being fixed if you intend to keep it as a single commit.
Attachment #8780157 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 5

2 years ago
Fair point, sorry! I've split off the cosmetic fixes into a separate patch for landing, so the first one contains just the single functional change. Carrying over your r+, as the total effect is the same.
(Assignee)

Comment 6

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e5c6bab845670af3b30c310145f3e9c8640d050
Bug 1294448 - Fix handling of platform ID in gfxFontUtils::ReadNames, so that fallback to Windows-platform names on Mac works as intended. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/0307f3b495ed7cc0105b338bcf1eb8b6175009ef
Bug 1294448 followup - Cosmetic fixes to code style, no functional change. r=jrmuizel
not critical enough for a 48 dot release, however, tracking to make sure we take a fix in 49
status-firefox48: affected → wontfix
tracking-firefox49: --- → +

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2e5c6bab8456
https://hg.mozilla.org/mozilla-central/rev/0307f3b495ed
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Hi :jfkthame,
Since this bug is a regression and also affects 49/50, are you also considering to uplift this patch to 49/50?
Flags: needinfo?(jfkthame)
(Assignee)

Comment 10

2 years ago
Considering this is a long-standing bug -- the visible "regression" in Firefox 44 was only that the placeholder "OTS derived font" was replaced by the more obviously broken "(MISSING NAME)", but even prior to that the correct font names were not being shown, AFAICT -- I don't think it really merits tracking for FF49/beta.

The patch is trivial enough that there's minimal risk to it, IMO, so I'll nominate it for aurora uplift.
Flags: needinfo?(jfkthame)
Target Milestone: mozilla51 → ---
(Assignee)

Comment 11

2 years ago
Comment on attachment 8780157 [details] [diff] [review]
Fix handling of platform ID in gfxFontUtils::ReadNames, so that fallback to Windows-platform names on Mac works as intended

Approval Request Comment
[Feature/regressing bug #]: bug 1193050 (but as noted above, even prior to that we were not actually showing the proper names)

[User impact if declined]: "MISSING NAME" for some remote fonts in Dev Tools inspector

[Describe test coverage new/current, TreeHerder]: tested manually with example from comment 0

[Risks and why]: minimal, trivial patch to font name-reading code

[String/UUID change made/needed]: none
Attachment #8780157 - Flags: approval-mozilla-aurora?
Based on comment #10, it's a long-standing bug. Mark 49 as won't fix.
status-firefox49: affected → wontfix
Comment on attachment 8780157 [details] [diff] [review]
Fix handling of platform ID in gfxFontUtils::ReadNames, so that fallback to Windows-platform names on Mac works as intended

The patch fixes a regression. Take it in 50 aurora.
Attachment #8780157 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 14

2 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/0463f6653d48
status-firefox50: affected → fixed
Verified that using Aurora 50.0a2 and Nightly 51.0a1 both from 2016-08-17, the font names are displayed under Mac OS X 10.9.5.
Status: RESOLVED → VERIFIED
status-firefox50: fixed → verified
status-firefox51: fixed → verified
You need to log in before you can comment on or make changes to this bug.