Closed Bug 1582016 Opened 3 months ago Closed 3 months ago

"text-decoration-skip-ink" interacts terribly with Guardian TextEgyp font (used on e.g. Engadget)

Categories

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

defect

Tracking

()

VERIFIED FIXED
mozilla71
Tracking Status
firefox70 --- verified
firefox71 --- verified

People

(Reporter: dholbert, Assigned: jfkthame)

References

()

Details

Attachments

(6 files)

STR:

  1. Load https://www.engadget.com/amp/2019/09/17/googles-fi-unlimited-plan/
  2. (optional) Zoom in with Ctrl+
  3. Look at underlined links in the article text.

ACTUAL RESULTS:
The underlines skip a lot, skipping over characters that don't have descenders. It's especially bad (i.e. it skips more) if you zoom in with Ctrl+.

EXPECTED RESULTS:
Only descenders should be skipped.

Chrome gives EXPECTED RESULTS. They're using the same webfont, "Guardian TextEgyp". This might be a difference in the choice of underline offset, or perhaps a different tolerance for closeness of decoration + text?

Calling this "P2" since we're about to ship this feature and Engadget is a pretty major site...

Flags: needinfo?(jfkthame)

Here's essentially the same testcase, with text-decoration-skip-ink:none, just so you can look at the placement of the underline in the absence of ink-skipping.

If you load this testcase in Firefox vs. Chrome, you can tell that the underline is much closer to the text in Firefox (particularly at high full-page-zoom levels).

Here's a screenshot showing testcase 2 in Firefox (no space between text+underline) vs. Chrome (nice clearly-defined space between text+underline).

This is the fault of the font. Dumping the 'post' table with TTX to check, we find:

  <post>
    <formatType value="2.0"/>
    <italicAngle value="0.0"/>
    <underlinePosition value="0"/>
    <underlineThickness value="102"/>
    ....etc

Note in particular underlinePosition value="0". This calls for the underline to be placed exactly at the baseline of the text. So no wonder it's liable to clash with most glyphs, and get broken up badly when skip-ink is used.

A simple fix for the site -- assuming it may take some time to actually get the font fixed -- would be to use text-underline-offset: 1px to shift the underline slightly below the baseline of the text.

(Chrome doesn't show the issue because it doesn't respect the underline position and thickness parameters provided by the font.)

Flags: needinfo?(jfkthame)

So what we should probably do to mitigate this issue is to clamp the offset to a reasonable minimum gap when text-underline-offset:auto is in effect, rather than entirely trusting the font's value.

This is to avoid skip-ink problems with fonts that leave the OpenType field at zero,
which is unlikely to ever be what is really wanted. Authors can still avoid the clamp by
explicitly using text-underline-offset:from-font.

Not sure if this will offend the reftest gods... I've pushed a try job to see how it looks: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ead75486cad6c4f62887f30d3241c26acc921dd3

We should uplift this to beta70 (where these features are enabled by default via bug 1573631), perhaps after this has gotten a day or so of testing on Nightly.

Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2f97bd10d6da
When text-underline-offset is `auto`, clamp the minimum used underline-offset to 1/16em. r=dholbert
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
Assignee: nobody → jfkthame

Comment on attachment 9093544 [details]
Bug 1582016 - When text-underline-offset is auto, clamp the minimum used underline-offset to 1/16em. r?dholbert

Beta/Release Uplift Approval Request

  • User impact if declined: text-decoration-skip-ink (new feature shipping in 70) leads to poor rendering of underlined text (e.g. links) on sites such as Engadget due to use of a webfont that does not have a proper underline-offset value.
    (Rendering was already somewhat poor with such fonts, due to the underline clashing with the baseline of glyphs, but with skip-ink enabled - which is the default setting - it becomes much worse; the underlining breaks up and virtually disappears.)
  • 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): Small patch to apply a minimum-offset clamp to the underline-offset value found in the font, so as to avoid clashing with glyphs in the case where the font doesn't provide a real value; no effect other than on underline positioning.
  • String changes made/needed:
Attachment #9093544 - Flags: approval-mozilla-beta?

Comment on attachment 9093544 [details]
Bug 1582016 - When text-underline-offset is auto, clamp the minimum used underline-offset to 1/16em. r?dholbert

Fix for text-underline-offset shipping in Fx70. Approved for 70.0b8.

Attachment #9093544 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

Hello!
Reproduced the issue using Firefox 71.0a1 (20190917155453) on Windows 10x64 using the test cases and the website mentioned.
The issue is verified fixed with Firefox 71.0a1 (20190922213341) and Firefox 70.0b8 (20190919164641) on Windows 10x64, macOS 10.14, Ubuntu 18.04.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.