1673098 - Betterment "Auto-deposit" text is shown with a weird flag instead of a hyphen

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect, P2)

Description

[Daniel Holbert [:dholbert]]

STR:

1. In an account at Betterment.com, view one of your "Goals" and look at the "Auto-Deposit" text

ACTUAL RESULTS:
There's a flag glyph instead of a hyphen, in the "Auto-Deposit" text. See attached screenshot.

EXPECTED RESULTS:
Hyphen.

Reduced testcase coming up.

Mozregression gives me this range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=97770f1880a215b81e1fe806f9d6e486246bfc69&tochange=6122ff2a25ce5c45a14d3b0cf0931f85aa9e4053

which points to this being regressed by bug 1670581.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: screenshot (see "Auto-deposit" text at bottom right)

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: testcase 1 (partially reduced, via "Testcase Reducer" add-on)

Comment 3

[Daniel Holbert [:dholbert]]

[Tracking Requested - why for this release]: newly-introduced regression

I'm using Nightly 84.0a1 (2020-10-23) (64-bit) on Ubuntu 20.04, for what it's worth.

Comment 4

[Jonathan Kew [:jfkthame]]

Huh, that's really weird. But it reproduces for me on macOS too. Will investigate.

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: further reduced testcase

Test file that loads the webfont from the site, and displays various dash/hyphen characters.

Here, we can see that U+2010 and U+2011, which should (per bug 1670581) fall back to rendering U+002D when they're not available in the chosen font, are instead rendering the flag glyph.

Comment 6

[Jonathan Kew [:jfkthame]]

Ah, I get it.... when it's a CFF font, we use the built-in harfbuzz font functions rather than our callbacks, and that means the hyphen fallback mappings don't get applied; the weird flag must be that font's .notdef glyph.

So we need to ensure we override the get_nominal_glyph function with our own version (where the fallback is implemented) for all fonts, including CFF.

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1673098 - Ensure we use our implementation of the get_nominal_glyph callback when shaping a CFF font, so that hyphen fallbacks can take effect. r=lsalzman

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0a6c18c6e9d5
Ensure we use our implementation of the get_nominal_glyph callback when shaping a CFF font, so that hyphen fallbacks can take effect. r=lsalzman

Comment 9

[Alexandru Michis [:malexandru]]

Backed out changeset 0a6c18c6e9d5 (Bug 1673098) for causing webrender failures.

Backout link: https://hg.mozilla.org/integration/autoland/rev/4847c70f7faa440dd17636336f18bca7ba8a4b14

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&fromchange=f08534dd3713d20de0090e7d16ae9a465ed891b3&searchStr=windows%2C7%2Copt%2Cweb%2Cplatform%2Ctests%2Ctest-windows7-32%2Fopt-web-platform-tests-reftest-e10s&tochange=d260f821050b6e66da57981bc3e94d4537d6f807&selectedTaskRun=aEU_0L6sRkWE7grvdQjjIA.0

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=319746146&repo=autoland&lineNumber=19399

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d91eaf128119
Ensure we use our implementation of the get_nominal_glyph callback when shaping a CFF font, so that hyphen fallbacks can take effect. r=lsalzman

Comment 11

[Alexandru Michis [:malexandru]]

Re-landed the changes that were backed out since the wpt-sync that followed looks to have fixed them somehow.
Re-land link: https://hg.mozilla.org/integration/autoland/rev/d91eaf1281199ecf04e36eea1d169e1bb39c371c

Comment 12

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/d91eaf128119

Comment 13

[Pascal Chevrel:pascalc (PTO until April 26)]

Jonathan, should we uplift this to Beta as this is an S2 bug and the fix seems to be small?

Comment 14

[Jonathan Kew [:jfkthame]]

Yes, I think it'd be good to uplift; the fix is simple and should be very safe.

Comment 15

[Jonathan Kew [:jfkthame]]

Comment on attachment 9183621 [details]
Bug 1673098 - Ensure we use our implementation of the get_nominal_glyph callback when shaping a CFF font, so that hyphen fallbacks can take effect. r=lsalzman

Beta/Release Uplift Approval Request

• User impact if declined: Unicode hyphens may be mis-rendered by some webfonts
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simple fix to ensure we apply a fallback character mapping to CFF as well as TTF fonts; should have no effect on any other characters than the Unicode hyphens we're targeting here.
• String changes made/needed:

Comment 16

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9183621 [details]
Bug 1673098 - Ensure we use our implementation of the get_nominal_glyph callback when shaping a CFF font, so that hyphen fallbacks can take effect. r=lsalzman

Approved for 83 beta 6, thanks.

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/cf6e1e2f1fbd