Skip to content

Proposal: load Ahem as webfont everywhere#16951

Closed
LukeZielinski wants to merge 1 commit intoweb-platform-tests:masterfrom
LukeZielinski:ahem-webfont
Closed

Proposal: load Ahem as webfont everywhere#16951
LukeZielinski wants to merge 1 commit intoweb-platform-tests:masterfrom
LukeZielinski:ahem-webfont

Conversation

@LukeZielinski
Copy link
Copy Markdown
Contributor

Update all tests using Ahem font to load it as a webfont. The fonts/ahem.css stylesheet is added to mark Ahem as a webfont, pointing to fonts/Ahem.ttf. The vast majority of the updated tests just link to this stylesheet.

The following 11 tests are non-reftests that use Ahem, so they also
explicitly preload the WebFont:

  • css/css-grid/grid-definition/grid-inline-support-flexible-lengths-001.html
  • css/css-grid/grid-definition/grid-inline-support-grid-template-columns-rows-001.html
  • css/css-grid/grid-definition/grid-inline-support-named-grid-lines-001.html
  • css/css-grid/grid-definition/grid-inline-support-repeat-001.html
  • css/css-grid/grid-definition/grid-inline-template-columns-rows-resolved-values-001.html
  • css/css-grid/grid-definition/grid-support-flexible-lengths-001.html
  • css/css-grid/grid-definition/grid-support-grid-template-columns-rows-001.html
  • css/css-grid/grid-definition/grid-support-named-grid-lines-001.html
  • css/css-grid/grid-definition/grid-support-repeat-001.html
  • css/css-grid/grid-definition/grid-template-columns-rows-resolved-values-001.html
  • css/css-shapes/spec-examples/shape-outside-018.html

One test also checks the list of resources that are loaded, so it was updated to expect
the ahem.css stylesheet to load as well:

  • resource-timing/resource_initiator_types.html

@LukeZielinski
Copy link
Copy Markdown
Contributor Author

@jgraham Hi James, what do you think of this change? Can you try it on Mozilla CI to see if it breaks anything?

@LukeZielinski
Copy link
Copy Markdown
Contributor Author

@foolip Regarding the check failure, a documented assumption is that Ahem is installed on the device. I guess we'd need to break this assumption, or relax it to say that Ahem is available as a webfont (and fix the test generator here)?

@cbiesinger cbiesinger removed their request for review May 22, 2019 17:10
@gsnedders
Copy link
Copy Markdown
Member

see also #9105 for earlier discussion of this

@gsnedders
Copy link
Copy Markdown
Member

@foolip Regarding the check failure, a documented assumption is that Ahem is installed on the device. I guess we'd need to break this assumption, or relax it to say that Ahem is available as a webfont (and fix the test generator here)?

If we're changing behaviour to always load Ahem as a web font, we have no need for that "assumption" and should drop it and the related test (don't even bother changing it: there's no point; it would just become a tautology that the web font is equal to the web font).

That said, more generally as I think was clear in #9105 I'm generally in favour of this but have never been able to justify anyone spending their time on it (so, uh, thanks!), but I do think we need an RFC for this change (which shouldn't be too hard to get through!).

I'd be interested in whether this made any notable difference to overall test execution time, but I'd hope the web font would get cached quite easily.

@cbiesinger
Copy link
Copy Markdown
Contributor

Don't you have to ensure to wait until the webfont is actually loaded?

@gsnedders
Copy link
Copy Markdown
Member

Don't you have to ensure to wait until the webfont is actually loaded?

One of the guarantees of WPT is that the reftest is taken after the document.fonts.ready promise resolves (though see also #11476).

@annevk annevk removed their request for review May 23, 2019 07:41
@drott
Copy link
Copy Markdown
Contributor

drott commented May 23, 2019

What's the intention of the change? To make the tests more robust due to issue with Ahem not being available from the system?

I don't have objections to this change in particular, but I think it'd be beneficial to improve our system font testing story, as outlined in #13203 (comment) or it would generally help if control over system fonts available to WPT became more reliable, or just .. available.

@wpt-pr-bot wpt-pr-bot requested a review from grorg May 23, 2019 12:11
@LukeZielinski
Copy link
Copy Markdown
Contributor Author

Regarding the motivation for this change: it's to allow us to run WPT on systems where it's unlikely or difficult to have a system font installed (specifically Chromium CI).

@LukeZielinski
Copy link
Copy Markdown
Contributor Author

I've created an RFC for this here: web-platform-tests/rfcs#22

@gsnedders
Copy link
Copy Markdown
Member

More generally, I think it's a big help for people running tests locally in their own browser, as that's a situation where it's much less likely they'll have Ahem installed versus (e.g.) fonts with decent coverage of characters.

@LukeZielinski
Copy link
Copy Markdown
Contributor Author

Discarding this PR, will split as per RFC discussion

@LukeZielinski LukeZielinski deleted the ahem-webfont branch June 14, 2019 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants