Closed Bug 1379444 Opened 2 years ago Closed 2 years ago

Mojibake/mangled EMOJI font after certain page displayed

Categories

(Core :: Graphics: Text, defect, P3)

Unspecified
Windows 7
defect

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox-esr52 --- fixed
firefox54 --- wontfix
firefox55 --- fixed
firefox56 --- fixed

People

(Reporter: alice0775, Assigned: jfkthame)

References

Details

(Keywords: regression, Whiteboard: [gfx-noted])

Attachments

(3 files, 1 obsolete file)

The problem seems to be only on Windows7.

Reproducible: always

Steps to reproduce:
1. Open https://en.wikipedia.org/wiki/Emoji#Unicode_blocks
   --- Observe, Emoji fonts are rendered as expected
2. Open https://en.wikipedia.org/wiki/Media_controls#Symbols
3. Open https://en.wikipedia.org/wiki/Emoji#Unicode_blocks again
   --- Observe, Emoji fonts are Mojibake/mangled --- BUG

Actual Results:
Emoji fonts are Mojibake/mangled

Expected Results:
No Mojibake
Keywords: regression
Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=024e7dc7a219e3f276bc2245746bbad67a574ea9&tochange=209072396aa5ab5c5a0a28109a980dbbcd884922

Regressed by:
209072396aa5	Mason Chang — Bug 1007702. Enable skia on unaccelerated windows. r=lsalzman
Blocks: skia-windows
Component: Layout: Text → Graphics: Text
Flags: needinfo?(mchang)
Attached file about_support.txt
I was able to reproduce this and am looking into it.
Assignee: nobody → lsalzman
Status: NEW → ASSIGNED
Flags: needinfo?(mchang)
Priority: -- → P3
Whiteboard: [gfx-noted]
It seems the reason the glyph changes how it is displayed is due to a change in link style. Initially, before the media controls page is visited, it is using the default link style. Then once the media controls page is visited, it switches to the a:visited style, which picks a different font.

At least the inspector does not show the font family being overridden beyond sans-serif. Trying to boil it down to a standalone test-case, I can't seem to get it to use EmojiOne Mozilla, but rather Arial, so I can only get it to happen in situ. The bigger mystery here is why initially we fall back to EmojiOne Mozilla inconsistently, and then later on, after the link is visited, we switch to Arial.

Still digging into this further.
If you toggle the pref "gfx.font_rendering.fallback.always_use_cmaps" to true, and restart the browser, does that make any difference?
(In reply to Jonathan Kew (:jfkthame) from comment #5)
> If you toggle the pref "gfx.font_rendering.fallback.always_use_cmaps" to
> true, and restart the browser, does that make any difference?

Yes, the pref fixes the problem.
It works as expected if set gfx.font_rendering.fallback.always_use_cmaps to true.
I think the issue here is probably that gfxDWriteFontList::GlobalFontFallback (which depends on using DirectWrite to shape the text and then checks what font it ends up using) fails to find the EmojiOne Mozilla font. (This issue is not unique to emoji; it can also fail with characters in "obscure" scripts that Windows didn't support, even if the user has installed additional fonts that do support them.) So we don't find a font, and show a missing-glyph hexbox instead.

The initially-correct display happens, I'm guessing, because the character in question is being shaped as part of a larger text run that includes emoji characters in preceding cells, and those were recognized by the fallback code and the emoji font chosen. And once we've chosen a fallback font, we try that as a first-guess for following characters in the same run, before hitting the GlobalFontFallback method again. So one successful fallback to EmojiOne can then "propagate" across the following characters.

However, after following the link and returning, the restyled text gets shaped separately as its own run, the propagation of the preceding fallback font choice doesn't apply, and GlobalFontFallback fails.

Setting gfx.font_rendering.fallback.always_use_cmaps solves this because then we don't rely on DirectWrite to shape text and find fallback fonts, we do our own search of the available fonts. The problem with this -- the reason the gfxDWriteFontList::GlobalFontFallback path was added -- is that it can be expensive, resulting in jank the first time it occurs because we have to load cmaps for all the fonts. (See bug 705594 for background here.)

However, IMO it would probably be preferable to do this, and risk the possibility of (one-time) jank, rather than risk failing to find a font for a character that in fact _is_ available in the user's installed fonts. Perhaps we should fall back to the cmap-searching code path in the case where gfxDWriteFontList::GlobalFontFallback fails, or perhaps check whether we have already loaded the font cmaps, and if so, prefer the cmap-searching path.
I think we should try doing something like this, so that if the platform-specific fallback (on dwrite and macos) fails to find a usable font, we'll do a last-resort search using the generic cmap-based code. There's a slight risk this could sometimes be slow the first time it occurs, if a lot of cmaps need to be loaded, but that should be very rare in practice; at worst, it's a one-time (per session) occurrence; and it's better than showing hexboxes for characters that are in fact available (as here).
Updated to fix the Windows build failure in that version. Try run is at https://treeherder.mozilla.org/#/jobs?repo=try&revision=c356ee8908eb50b482fe21031017ae39c58a2d75; Alice/Lee, if you could test this build and confirm whether it fixes the issue, that would be helpful - thanks.
Attachment #8885723 - Attachment is obsolete: true
(In reply to Jonathan Kew (:jfkthame) from comment #9)
> Try run is at
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=c356ee8908eb50b482fe21031017ae39c58a2d75; Alice/Lee,
> if you could test this build and confirm whether it fixes the issue, that
> would be helpful - thanks.

I conform that the try build fixes the problem :)
Attachment #8885741 - Flags: review?(lsalzman)
Attachment #8885741 - Flags: review?(lsalzman) → review+
Assignee: lsalzman → jfkthame
https://hg.mozilla.org/integration/mozilla-inbound/rev/4699fe6d75de9d544b573c7e425eacb5830786ac
Bug 1379444 - Use generic cmap-based font fallback if platform-specific code fails to find a usable font. r=lsalzman
https://hg.mozilla.org/mozilla-central/rev/4699fe6d75de
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Please request Beta and ESR52 approval on this when you get a chance (ESR52 mainly because we have more Skia users there due to XP/Vista).
Flags: needinfo?(jfkthame)
Comment on attachment 8885741 [details] [diff] [review]
Use generic cmap-based font fallback if platform-specific code fails to find a usable font

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
With the switch to Skia rendering on older Windows systems, we are using the DirectWrite font back-end more widely, which means users who previously were unaffected by this issue (their systems used GDI fonts) may now see it.

User impact if declined: potential for emoji (or other "rare" unicode characters) to render as missing-glyph hexboxes even though a suitable font is available on the system

Fix Landed on Version: 56

Risk to taking this patch (and alternatives if risky): Minimal functional risk, this just falls back to our pre-existing "exhaustive" font fallback search if the DirectWrite fallback fails. There is a possibility of a one-time performance hit, the first time font fallback kicks in during a session and DirectWrite fails to find a usable font.

String or UUID changes made by this patch: none



Approval Request Comment
[Feature/Bug causing the regression]: switch to Skia/DirectWrite backend

[User impact if declined]: see above

[Is this code covered by automated tests?]: the code will be exercised during existing reftests, but the exact details of font fallback behavior are too dependent on system configuration (installed fonts, etc) to be reliably tested in automation

[Has the fix been verified in Nightly?]: yes

[Needs manual test from QE? If yes, steps to reproduce]: no

[List of other uplifts needed for the feature/fix]: none

[Is the change risky?]: not really, see above

[Why is the change risky/not risky?]: see above

[String changes made/needed]: none
Flags: needinfo?(jfkthame)
Attachment #8885741 - Flags: approval-mozilla-esr52?
Attachment #8885741 - Flags: approval-mozilla-beta?
Comment on attachment 8885741 [details] [diff] [review]
Use generic cmap-based font fallback if platform-specific code fails to find a usable font

tweak font fallback to be more likely to find a suitable font, beta55+
Attachment #8885741 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Jonathan Kew (:jfkthame) from comment #14)
> [Is this code covered by automated tests?]: the code will be exercised
> during existing reftests, but the exact details of font fallback behavior
> are too dependent on system configuration (installed fonts, etc) to be
> reliably tested in automation
> 
> [Has the fix been verified in Nightly?]: yes
> 
> [Needs manual test from QE? If yes, steps to reproduce]: no

Setting qe-verify- based on Jonathan's assessment on manual testing needs and the fact that the code affected by this fix is exercised by existing reftests.
Flags: qe-verify-
Comment on attachment 8885741 [details] [diff] [review]
Use generic cmap-based font fallback if platform-specific code fails to find a usable font

Fix a regression related to emoji for ESR users. Let's uplift it to ESR52.3.
Attachment #8885741 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
You need to log in before you can comment on or make changes to this bug.