New issue
Advanced search Search tips

Issue 696867 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug

Blocked on:
issue 731563



Sign in to add a comment

Mac: Harfbuzz fallback fonts don't always match native/Blink (e.g. for Hebrew)

Project Member Reported by tapted@chromium.org, Feb 28 2017

Issue description

Chrome Version       : 58.0.3018.3
OS Version: OS X 10.12.3

What steps will reproduce the problem?
1. On Mac, Navigate, e.g., to https://he.wikipedia.org/wiki/%D7%A1%D7%99%D7%91%D7%99%D7%AA (or Google for "סיבית") [see screenshots]
2. Switch your OS Language between English/Hebrew and restart [observer differences in WebContents]

or

1. Enable chrome://flags/#secondary-ui-md
2. Open bookmark bubble, paste "סיבית" into a textfield
3. Paste the same text into the omnibox

What is the expected result?

Fonts should match

What happens instead of that?

The rightmost glyph is different. Native NSTextFields render it "rounder", regardless of the operating system language. But
 - Blink renders it "round" only when the OS Language is set to Hebrew.
 - RenderTextHarfbuzz never renders it "round" (regardless of OS Language)
 - Safari never renders it "round" either (regardless of OS Language).

This is a problem when trying to switch to Harfbuzz everywhere. The minor kerning differences between RenderTextHarfbuzz and RenderTextMac on the same font probably won't be noticed. However, a totally different font will badly break things like gfx::ElideText and gfx::GetStringWidth[F]

I'm not sure who's doing it right.. Generally we assume NSTextField does the right thing, in which case
 - Blink shouldn't change fonts when the OS Language changes
 - RenderTextHarfbuzz needs a different font fallback source.


If we instead assume Blink is doing it right, then
 - RenderTextHarfbuzz needs to get its font fallbacks from the same place, and
 - To switch to Harfbuzz, we need to convince gfx::ElideText/GetStringWidth either to "Agree" with NSTextField, or be aware of whether they're eventually going to be painted using Skia or AppKit
 
chipboard-hebrew.png
44.8 KB View Download
chipboard-english.png
48.4 KB View Download

Comment 1 by tapted@chromium.org, Feb 28 2017

Description: Show this description
Blockedon: 731563
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/62cbc10d4dbfeed614eb23c3d996793814a35334

commit 62cbc10d4dbfeed614eb23c3d996793814a35334
Author: tapted <tapted@chromium.org>
Date: Wed Jun 21 01:26:13 2017

Use CTFontCreateForString for RenderTextHarfbuzz on Mac

The approach ends up pretty close to what's already done for Windows.

On Mac, CTFontCreateForString() gives better fallbacks (i.e. they more
often match what native UI does) and requires RenderTextHarfbuzz to do
less work. However, it doesn't guarantee that the returned font can
display every glyph in the provided string.

So still use CTFontCopyDefaultCascadeListForLanguages (which is now a
public API - hooray) to provide a last-resort list of fallback fonts.

BUG= 696867 ,  619684 

Review-Url: https://codereview.chromium.org/2927953003
Cr-Commit-Position: refs/heads/master@{#481062}

[modify] https://crrev.com/62cbc10d4dbfeed614eb23c3d996793814a35334/ui/gfx/font_fallback.h
[modify] https://crrev.com/62cbc10d4dbfeed614eb23c3d996793814a35334/ui/gfx/font_fallback_mac.mm
[modify] https://crrev.com/62cbc10d4dbfeed614eb23c3d996793814a35334/ui/gfx/font_fallback_mac_unittest.cc
[modify] https://crrev.com/62cbc10d4dbfeed614eb23c3d996793814a35334/ui/gfx/font_fallback_win.cc
[modify] https://crrev.com/62cbc10d4dbfeed614eb23c3d996793814a35334/ui/gfx/font_fallback_win.h
[modify] https://crrev.com/62cbc10d4dbfeed614eb23c3d996793814a35334/ui/gfx/render_text_harfbuzz.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dca011e454153e08602ead192cda84abc2946a64

commit dca011e454153e08602ead192cda84abc2946a64
Author: tapted <tapted@chromium.org>
Date: Wed Jun 21 02:58:07 2017

Revert of Use CTFontCreateForString for RenderTextHarfbuzz on Mac (patchset #5 id:80001 of https://codereview.chromium.org/2927953003/ )

Reason for revert:
Causes views_unittest LabelSelectionTest.MouseDragSingleLineRTL to fail on 10.10 and 10.11

Original issue's description:
> Use CTFontCreateForString for RenderTextHarfbuzz on Mac
>
> The approach ends up pretty close to what's already done for Windows.
>
> On Mac, CTFontCreateForString() gives better fallbacks (i.e. they more
> often match what native UI does) and requires RenderTextHarfbuzz to do
> less work. However, it doesn't guarantee that the returned font can
> display every glyph in the provided string.
>
> So still use CTFontCopyDefaultCascadeListForLanguages (which is now a
> public API - hooray) to provide a last-resort list of fallback fonts.
>
> BUG= 696867 ,  619684 
>
> Review-Url: https://codereview.chromium.org/2927953003
> Cr-Commit-Position: refs/heads/master@{#481062}
> Committed: https://chromium.googlesource.com/chromium/src/+/62cbc10d4dbfeed614eb23c3d996793814a35334

TBR=asvitkine@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 696867 ,  619684 

Review-Url: https://codereview.chromium.org/2950013002
Cr-Commit-Position: refs/heads/master@{#481095}

[modify] https://crrev.com/dca011e454153e08602ead192cda84abc2946a64/ui/gfx/font_fallback.h
[modify] https://crrev.com/dca011e454153e08602ead192cda84abc2946a64/ui/gfx/font_fallback_mac.mm
[modify] https://crrev.com/dca011e454153e08602ead192cda84abc2946a64/ui/gfx/font_fallback_mac_unittest.cc
[modify] https://crrev.com/dca011e454153e08602ead192cda84abc2946a64/ui/gfx/font_fallback_win.cc
[modify] https://crrev.com/dca011e454153e08602ead192cda84abc2946a64/ui/gfx/font_fallback_win.h
[modify] https://crrev.com/dca011e454153e08602ead192cda84abc2946a64/ui/gfx/render_text_harfbuzz.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7d814671483a3cde6b0c9e7c616ceb1eebfd3f17

commit 7d814671483a3cde6b0c9e7c616ceb1eebfd3f17
Author: tapted <tapted@chromium.org>
Date: Fri Jun 23 01:37:09 2017

Use CTFontCreateForString for RenderTextHarfbuzz on Mac

The approach ends up pretty close to what's already done for Windows.

On Mac, CTFontCreateForString() gives better fallbacks (i.e. they more
often match what native UI does) and requires RenderTextHarfbuzz to do
less work. However, it doesn't guarantee that the returned font can
display every glyph in the provided string.

So still use CTFontCopyDefaultCascadeListForLanguages (which is now a
public API - hooray) to provide a last-resort list of fallback fonts.

This CL causes a font used for Hebrew text in a test to change, which
tickled a pre-existing bug/corner case in the test for some macOS
versions.

BUG= 696867 ,  619684 ,  735346 

Previously landed in r481062, but tickled some OS-specific tests due to
a valid discrepancy between rounding directions.

Review-Url: https://codereview.chromium.org/2927953003
Cr-Commit-Position: refs/heads/master@{#481764}

[modify] https://crrev.com/7d814671483a3cde6b0c9e7c616ceb1eebfd3f17/ui/gfx/font_fallback.h
[modify] https://crrev.com/7d814671483a3cde6b0c9e7c616ceb1eebfd3f17/ui/gfx/font_fallback_mac.mm
[modify] https://crrev.com/7d814671483a3cde6b0c9e7c616ceb1eebfd3f17/ui/gfx/font_fallback_mac_unittest.cc
[modify] https://crrev.com/7d814671483a3cde6b0c9e7c616ceb1eebfd3f17/ui/gfx/font_fallback_win.cc
[modify] https://crrev.com/7d814671483a3cde6b0c9e7c616ceb1eebfd3f17/ui/gfx/font_fallback_win.h
[modify] https://crrev.com/7d814671483a3cde6b0c9e7c616ceb1eebfd3f17/ui/gfx/render_text_harfbuzz.cc
[modify] https://crrev.com/7d814671483a3cde6b0c9e7c616ceb1eebfd3f17/ui/views/controls/label_unittest.cc

Comment 6 by tapted@chromium.org, Jun 27 2017

Blocking: -454835
Components: -Internals>Views
Using CTFontCreateForString in gfx::RenderTextHarfBuzz now means there's a perfect match between RTHB and the native strings I've tested. Updating tags/blockers.

The font used for webcontents still changes depending on the OS Language setting, whereas CTFontCreateForString and Safari / Cocoa return the same Font, e.g., for סיבית regardless of whether the OS language is English or Hebrew.
fallback-rthb.png
77.6 KB View Download
Project Member

Comment 7 by sheriffbot@chromium.org, Jun 27 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 8 by e...@chromium.org, Jun 28 2018

Status: Fixed (was: Untriaged)

Sign in to add a comment