New issue
Advanced search Search tips

Issue 763666 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 3
Type: Bug



Sign in to add a comment

LayoutTests/external/wpt/assumptions/ahem.html fails on Linux and Windows

Project Member Reported by foolip@chromium.org, Sep 9 2017

Issue description

Trying to enable some more tests in https://chromium-review.googlesource.com/c/chromium/src/+/658979 I found that this test fails in Linux and Windows, but not on Mac.

It's possible that other tests that depend on ahem are failing due to the same root cause.
 
The test result means that Ahem is not installed on our Linux and Windows bots, but installed on the Mac bot. (I also quickly checked the test itself and it seems correct.)

Where do we set up the try bots?

Comment 2 by foolip@chromium.org, Sep 14 2017

It looks like Ahem is installed, but there's some problem with the position of two glyphs, attaching the diffs from the bots.
ahem-diff-win.png
4.0 KB View Download
ahem-diff-linux.png
4.0 KB View Download
Cc: robertma@chromium.org
Status: Available (was: Untriaged)
Although I still don't know how we install Ahem fonts on bots, I've confirmed the issue and found the root cause being version mismatch.

The Ahem font installed on bots seems to be the older version 1.1, but the reference font in external/wpt/css/fonts/ahem is version 1.2. I can reproduce similar image diff by installing v1.1 on my machine and running this reftest.

In LayoutTests, we have the following copies of Ahem:

8cdc9e68594fbb6db8c7b4bff643ab2432b51db6  ./resources/Ahem.ttf
8cdc9e68594fbb6db8c7b4bff643ab2432b51db6  ./http/tests/resources/Ahem.ttf
8cdc9e68594fbb6db8c7b4bff643ab2432b51db6  ./http/tests/w3c/webperf/resources/Ahem.ttf
8cdc9e68594fbb6db8c7b4bff643ab2432b51db6  ./external/wpt/content-security-policy/support/Ahem.ttf
88d834aadcd7b547720992a2894b9ab3251e3e04  ./external/wpt/css/fonts/ahem/ahem.ttf
8cdc9e68594fbb6db8c7b4bff643ab2432b51db6  ./external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/support/Ahem.ttf
8cdc9e68594fbb6db8c7b4bff643ab2432b51db6  ./external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/variables/support/Ahem.ttf
8cdc9e68594fbb6db8c7b4bff643ab2432b51db6  ./external/wpt/fonts/Ahem.ttf
8cdc9e68594fbb6db8c7b4bff643ab2432b51db6  ./inspector-protocol/css/resources/Ahem.ttf

Only the one in external/wpt/css/fonts/ahem is the latest 1.2. And I suspect resources/Ahem.ttf is the one used on our bots (though, again, I don't know how bots are set up).

Also filed https://github.com/w3c/web-platform-tests/issues/7352 for why we have so many copies of Ahem in WPT.
Cc: qyears...@chromium.org
Found the upstream repo of Ahem (thanks to gsnedders): https://github.com/litherum/AhemMaker and the current latest version is 1.5.

The mismatch is related to a change in Ahem https://github.com/litherum/AhemMaker/commit/5032df7d0f1a8d03bf03d86a7bff29be53853a49

Anyway, the problem should be fixed once we update our Ahem version. +qyearsely, do you have insights on how Ahem is installed on try bots?
Cc: -qyears...@chromium.org
Owner: qyears...@chromium.org
Status: Started (was: Available)
So, I'm not entirely sure how fonts are configured, but it looks like there's another copy of Ahem.ttf in //content/shell/test_runner/:
 https://cs.chromium.org/chromium/src/content/shell/test_runner/resources/fonts/

And it seems like when we change that font, then external/wpt/assumptions/ahem.html passes. I just uploaded a CL after checking locally: https://crrev.com/c/668683
By the way, Quinten, you might want to wait until the upstream issue (https://github.com/w3c/web-platform-tests/issues/7352) comes to a resolution to see if we want to unify all these Ahem fonts and directly jump to the latest 1.5.
Good point; sounds like a plan.
Upstream PR has been merged. The resolution is to unify all copies to the latest version 1.50. See https://github.com/w3c/web-platform-tests/pull/7372

external/wpt will be synced, and we just need to deal with ahem.ttf outside the wpt directory.

A reminder: there is this super hack in LayoutTests/resources/ahem.js which contains hexified ahem.ttf in an array to force synchronised loading, which also needs to be updated.
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 10 2017

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

commit 0557b13e5a8e90c420a422ca485d24f4b28a915e
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Tue Oct 10 20:04:33 2017

Update content_shell's Ahem font to v1.5.

This CL copies the Ahem.ttf from the copy at:
//third_party/WebKit/LayoutTests/external/wpt/fonts/Ahem.ttf

The purpose of this is to fix external/wpt/assumptions/ahem.html.

Bug:  763666 
Change-Id: Ibff654884b7803b371104c030f47bf9a48a3fc5f
Reviewed-on: https://chromium-review.googlesource.com/668683
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Robert Ma <robertma@chromium.org>
Reviewed-by: Rick Byers <rbyers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507747}
[modify] https://crrev.com/0557b13e5a8e90c420a422ca485d24f4b28a915e/content/shell/test_runner/resources/fonts/AHEM____.TTF
[modify] https://crrev.com/0557b13e5a8e90c420a422ca485d24f4b28a915e/third_party/WebKit/LayoutTests/TestExpectations

Status: Fixed (was: Started)

Sign in to add a comment