[css-text-3] Line breaking tests #1135
Conversation
9021b98 to
b87cec2
Compare
|
@gsnedders The above check should be amended, since we no longer require explicit author information in the test (as it is provided by the version control system). |
|
The syncbot messages are coming from Shepherd. Have we formally relaxed the author requirement? Last time I recall it being discussed VCS info wasn't considered sufficient as some tests come from other sources. |
|
I thought we had relaxed it, and was trying to do things the new way to see how it would go. If we still want the author in there, I don't mind putting it in. If we have relaxed this requirement, here's an opportunity to do something about it. |
Automatic validation checks of commit 01dc109 passed. |
|
Simplified the tests a little bit, since we concluded in w3c/csswg-drafts#567 (comment) that wrap opportunities between ideographic characters was a normative requirement that we could depend on. |
|
This PR would be a great opportunity for you to demonstrate how GitHub allows you to rewrite your PR to reflect the final logical changesets that should end up on the trunk rather than a stack partially-finished work, right? Right? :) Or can we still not do that? |
01dc109 to
b312024
Compare
Automatic validation checks of commit b312024 passed. |
|
@fantasai: We can collapse history. Whether we should is a matter of personal taste. Since you're reviewing and you prefer it that way, I just did. |
Automatic validation checks of commit bc891bd passed. |
|
@fantasai @gsnedders @kojiishi Review time? |
| <link rel=author title="Florian Rivoal" href="http://florian.rivoal.net"> | ||
| <!-- | ||
| This is reference is theoretically identical to line-breaking-001.html, | ||
| but is needed nonetheless due to difference in anti-aliasing / font rendering. |
There was a problem hiding this comment.
Um, how? They're different fonts and different glyphs?
There was a problem hiding this comment.
I can get rid of the comment if it bothers you, but it ought to be true because:
- both tests have
font-size: 20pxandline-height: 1em, so both have a line-height of 20px - the
口character is a CJK ideograph, and therefore its advance measure can be expected to be 1em in 99% of the fonts. It could be different, but in practice it will not be. - If you combine the two points above, in line-breaking-ic-001-ref.html, each
口is exactly a 20px square. - The ink of of
口is of course smaller than that, but it does not matter since the background color is set as well. - In the X letter in ahem is defined to be a square exactly 1em in height and width.
- Both files then vertically stack two of these boxes.
=> Both files should result in a 20px × 40px vertical green rectangle.
In the case of line-breaking-ic-001-ref.html, this is what you get. In the case of line-breaking-001-ref.html, on standard resolution screens, you get that as well, and on high-res screen, you get approximately that modulo some fuzziness in the rendering of Ahem, probably for the same reasons we're having issues with the writing-modes test suite.
All the tests that use either reference use the same font (Ahem, or default CJK + background) as the reference they are pointing to, so we should not have the issues that the writing modes test suite is having. But if this Ahem fuzzyness issue was solved, we would be able to have a single ref instead of two.
There was a problem hiding this comment.
Also, while we're at it, you made me notice that I wrote "identical to line-breaking-001.html" instead of "identical to line-breaking-001-ref.html", so I added a commit to fix just that.
Automatic validation checks of commit f080c18 passed. |
|
@gsnedders Thanks for the review. I've explained the comment you found suspicious. Let me know if that's good enough, or if you'd still like something to be done about it. |
|
@frivoal I'd rather something like "as a CJK ideograph, its advance is expected to be 1em, and as both background and color are green it should typically render identically to line-breaking-001-ref". I'll point out we don't require specific fonts, and it's not implausible that no CJK fonts will be installed (older versions of Windows don't include any CJK fonts by default, AFAIK, and I think the same is true for some Linux distros). |
Automatic validation checks of commit cbb0986 passed. |
|
nit: not critical at all, but CSS Orientation Test Font can be a i18n-friendly, Ahem-like font, though its name doesn't suggest so. The font is already imported in the test directory, I use it in my several tests. |
|
Good to know, thanks. I'll keep it in mind. |
A few tests for https://www.w3.org/TR/css-text/#line-break-details
Note that the line-breaking-ic-*.html test are a bit more complex that should be strictly necessary, due to the fact that w3c/csswg-drafts#567 isn't resolved. The 3 tests should still be correct, but they could be simplified if the line breaking behavior between characters with the IC character class was made normative as requested in that bug.
This change is