Added line-breaking-013 and reference test #13027
Conversation
wpt-pr-bot
requested review from
fantasai,
frivoal,
hakatashi,
kojiishi,
nox,
plinss and
r12a
There was a problem hiding this comment.
Overall, this looks good, but:
- please split each block (1/2/3/4) into separate tests files. They are testing related but distinct things, and standalone tests make that easier to work with. Adjust the assert meta accordingly (for instance by combining the existing assert meta with the relevant comment for each block).
- please further split each sub-case of each block (image vs inline block) into separate test files.
- is the third subcase of each block (with 国) meant to be a test, or just a handy reference? If it is a test (which seems reasonable), it should have its own test file and test assertion.
- Please also address the comments made inline
- using html instead of xhtml is preferred. It allows skipping a number of closing tags (
</p>), elements (<head>,<body>,<html>), or attributes (xmlns="http://www.w3.org/1999/xhtml"), which makes the whole file shorter and easier to read and review. Using xhtml is not forbidden, so I'll still accept the test if it stays that way, but html would be nicer.
| display: inline-block; | ||
| width: 1em; | ||
| height: 1em; | ||
| background-color: green; |
There was a problem hiding this comment.
Don't use the color green unless its presence (together with the absence of red) means that the test passes.
As this is not the case here, you can use blue just like the inline image. If you want to maintain a distinction, orange is also commonly used as a color which does not by itself imply passing of failing.
| <span>a b c国d e f</span> | ||
| </div> | ||
|
|
||
| <!-- Second block - a breakpoint is normally disallowed between a numeric prefix like "+" and an ideographic character --> |
There was a problem hiding this comment.
UAX14 disallows breaks between numeric prefixes and ideograph, but following this rule is not strictly required by css-text. Some of the UAX14 rules are explicitly invoked and must be honored, but for the rest, css-text treats UAX14 more as a general guideline / starting point than as strict rules to be followed.
So I don't think this "block two" part of the text is wrong, but it also doesn't rise to the level of an RFC2119 “MUST”.
As this test is split into several files, those that correspond to "block two" should have <meta name="flags" content="should"> in the headers, as well as an additional <link rel="help" href="… pointing to the relevant section of UAX14.
| } | ||
| .test { | ||
| margin: 0 0 1em 0; | ||
| line-height: 1.25; |
There was a problem hiding this comment.
line-height and vertical align don't seem essential for the test. I suggest dropping them to simplify, and to avoid misleading people into thinking that they might matter somehow.
| font-size: 32px; | ||
| display: inline-block; | ||
| margin-right: 1em; | ||
| vertical-align: top; |
There was a problem hiding this comment.
vertical-align doesn't seem essential for the test. I suggest dropping it to simplify, and to avoid misleading people into thinking that it might matter somehow.
| <meta name="assert" content="Atomic inlines are treated as ideographic characters for line-breaking" /> | ||
| <link rel="match" href="reference/line-breaking-013-ref.xht" /> | ||
| <style> | ||
| p { |
There was a problem hiding this comment.
Styling the <p> doesn't seem essential for the test. I suggest dropping it to simplify, and to avoid misleading people into thinking that it might matter somehow.
Added test for line breaking behaviour discussed in w3c/csswg-drafts#3085 (comment)