[css-fonts] Test font style matching clauses 4 a-c #810
Conversation
This test exercises the font style matching clauses 4. a-c by iterating over all possible combinations of stretch, style and weight. It declares a @font-face rules and a style declaration for each combination, then uses it in the HTML. Each @font-face rule refers to a different ttf files with a special property: it contains ligatures for the words stretch, style and weight and replaces those words with the respective value. This allows verification of whether the UA selected the right font face rule and loaded the right corresponding ttf. The test fonts were generated using custom scripts and the fonttools library by subsetting, reducing and modifying the ligatures lookup in the Linux Libertine Font, which is dual licensed under GPL3 and OFL.
Automatic validation checks of commit ffaaa4d discovered the following problems:In css-fonts-3/font-style-matching-manual.html:
|
|
Based on this this PR I'd like to discuss and hear feedback on how we can better test the font style matching clauses 4 a-c. The test right now is manual and a bit verbose, all expanded combinations are spelled out - this could be simplified using JS and CSS OM or similar. It is a start however, highlighting issues with I am curious to find out how we could improve the testing in this regard:
I plan to extend the testing to check for correct precedence, by reducing the set of available fonts, so that the UA has to find the closest matching font while adhering to the precedence of stretch, style, weight. |
|
This is an interesting test; nice job on subsetting the fonts and the license for the font is fine for this. To explain the error reported by the bot, it is complaining about the missing metadata (such as a meta element to say this is actualy testing the specific part of the spec you mention in prose). Documentation is here: |
|
@svgeesus because this is using web fonts this doesn't require the font flag (which means fonts must be installed for the test to be run) My advice for automating it would be to give the fonts multiple family names. e.g., for |
syncbot
force-pushed
the
master
branch
2 times, most recently
from
d2eb706
to
9ee3de7
Compare
|
@svgeesus If the metadata were fixed, would these tests be correct and ready to merge? |
|
@svgeesus Because adding in the metadata is relatively trivial, and could be done by whoever merges in this changeset. |
I think so, but will check. Thanks for the reminder! |
| font-size: 24px; | ||
| line-height: 110%; | ||
| text-rendering: optimizeLegibility; | ||
| font-feature-settings: "liga" 1; |
There was a problem hiding this comment.
Should you turn on font-variant-ligatures too?
|
|
||
|
|
||
|
|
||
| #semicondensed_oblique_sixhundred { |
There was a problem hiding this comment.
Why do these this start with whitespace?
| } | ||
|
|
||
| @font-face { | ||
| font-family: CSSMatchingTest; |
There was a problem hiding this comment.
I don't quite understand this test. The important piece of the algorithm is what gets selected when the weight × width × slope space is sparse. It looks like you are populating the entire space with fonts, so I'm a little confused how these tests actually work.
| matching algorithm</a>. Stretch, style and weight information specified on the left-hand side should match the | ||
| information on the right-hand side.</p> | ||
| <p>This test works by declaring @font-face rules for each combination of stretch, style and weight and refering to a | ||
| different src url for each such rules. The font that the src url points to has a special property: it renders |
There was a problem hiding this comment.
Because you built these fonts with a tool, I think it would be a good idea to check in the source input file that the tool ingests (so people can see how you built it).
There was a problem hiding this comment.
Agreed that it would be much easier to review the tool than the somewhat verbose tests it generated.
| selected font can be verified by checking whether the correct ligatures were rendered. If they match, the user | ||
| agent matched the right font-face rule and loaded the correct respective ttf file for rendering.</p> | ||
| <table> | ||
| <tr><td >normal_normal_onehundred</td><td id="normal_normal_onehundred">stretch style weight</td></tr> |
There was a problem hiding this comment.
I'm a little new to this test infrastructure, but where are the expected results? If this is a reftest, don't you have the problem where there is so much content that it will overflow the viewport and not be tested? If this isn't a reftest, how are you using ligatures to test the results?
There was a problem hiding this comment.
Right, reftests are supposed to render identically in a 600x600 viewport
http://web-platform-tests.org/writing-tests/reftests.html
implying that this should be split into a bunch of smaller tests.
Sorry for not commenting earlier. I am not sure whether this should be landed as is. The version I uploaded is so far a manual test and not necessarily that useful for automated testing. In Blink, we have a mechanism for checking the font name of the used font in a layout test harness, e.g. I can select a Alternatively, this test could be turned into a reftest by explicitly selecting the right font in the reference rendering. @litherum said:
Very true. In my comment after uploading I mentioned:
This is something I've done in the meantime, in the Blink repo: There, the matching tests are run against a restricted set of fonts and the font style matching algorithm has to find the closest, in all situations of availability. Since in Blink the test is self validating just through the selected font family name, I do not run into viewport issues. I also split the test into 9 tests to keep the execution time under control.
I built them with fonttools, and IIRC I wrote a custom python script to output the TTXes. I can see whether I still have the generator script. However, with regards to the TTXes that fonttools works on: In Blink reviews we usually decided to not upload TTX source files since fonttools is only a disassembler/assembler and those files do not contain anything on top of the binary font file.
Yes.
We can remove the indentation. |
|
Note this PR has not got moved over to web-platform-tests; the review comments make it awkward to move. My recommendation is that this branch is rebased on top of the (last ever) csswg-test master, all further work is done on this branch, the PR is reviewed here, and then let me go and I'll merge it into web-platform-tests. |
Also, note the TTX format has no compatibility guarantees: it can and does get broken between fonttools releases. |
|
So, it seems that there is agreement that
Just checking we are on the same page regarding next steps. |
on the other hand, the generator python script seems like it would be useful to commit if it's still around |
This test exercises the font style matching clauses 4. a-c by iterating
over all possible combinations of stretch, style and weight. It declares
a @font-face rules and a style declaration for each combination, then
uses it in the HTML.
Each @font-face rule refers to a different ttf files with a special
property: it contains ligatures for the words stretch, style and weight
and replaces those words with the respective value.
This allows verification of whether the UA selected the right font face
rule and loaded the right corresponding ttf.
The test fonts were generated using custom scripts and the fonttools
library by subsetting, reducing and modifying the ligatures lookup in
the Linux Libertine Font, which is dual licensed under GPL3 and OFL.