On 3/17/23 02:02, Koji Ishii wrote:
On Fri, Mar 17, 2023 at 4:40 AM fantasai <fantasai.li...@inkedblade.net
<mailto:fantasai.li...@inkedblade.net>> wrote:
That test doesn't cover the interaction of other values of `white-space`
with
the `nowrap` value of `text-wrap`. It would be incorrect for e.g.
`white-space: pre` to return `text-wrap: wrap`.
I think it would be better to just implement both longhands of `white-space`
properly.
Implementing `nowrap` and other values is easy but it creates other new compat
risks that are more difficult for authors to handle.
Can you outline these new compat risks that only apply to `nowrap`? Also, are
they filed as an issue against the CSS spec? If there is a real compat risk to
implementing the spec, then the spec needs to be changed.
Note that if the spec changes to not have a shorthanding relationship due to
compat risks, then that means your implementation would need to change to stop
faking a shorthand relationship also.
Going back to that first quoted point: you are currently not testing the value
of `text-wrap` when `white-space: pre` has been declared. If you are returning
`text-wrap: wrap` rather than `text-wrap: nowrap`, then we have a problem,
because that is not compatible with what the spec requires.
Since you think this is the correct thing to do, can you explain why you think
it is better for Blink to ship a behavior change in the future (to match the
spec) vs. shipping the correct behavior initially?
Btw, I notice you are only testing the shorthanding side-effects you have
carefully faked in your code, rather than all of them. I know it's natural to
only write (passing) tests for things you've specifically implemented, but it
is misleading... because if Blink intends to ever follow the spec, there are
property interactions here that will noticeably change. And your reviewers
should be made aware of that.
With the current impl, authors can detect the situation by
`supports(‘text-wrap: nowrap’)`.
I don't understand what this suggestion is solving.
I'm not concerned about the quality of the balancing, as I'm sure it's fine,
and it will improve over time... my concerns are mainly with
a) interaction of the properties
b) any layout interactions with e.g. floats, initial-letter, positioning,
text justification, box sizing, etc.
c) whether the CSSWG considers this stable enough to ship, or if there are
unresolved concerns about the design of the feature
I understand that the spec is still in early draft, it’s WD, not CR, PR, nor
REC. If the spec changes, we can change our implementation accordingly.
As far as I understand, once Blink ships something, whether it is willing to
change its implementation generally depends on the usage rates of the new
feature. So if Blink ships the feature, and it is widely used, then Blink
would be unwilling to change or drop the feature. Does that not apply here?
Also, why not ask the CSSWG about (c)?
I'll note that there was an issue filed in the CSSWG repo recently in
response
to the Blink implementation:
https://github.com/w3c/csswg-drafts/issues/8516
<https://github.com/w3c/csswg-drafts/issues/8516>
and it raises questions about sizing and floats, among other things.
Thank you for citing this, yes, that was great feedback. I fixed bugs reported
there, Tab and I responded, and the reporter responded “agree that this is the
best approach
<https://github.com/w3c/csswg-drafts/issues/8516#issuecomment-1453802827>”.
The reporter was agreeing specifically with adjusting the limit to 4 lines
rather than 10, not with anything else. Some of the other points brought up by
the reporter include:
* Interaction with `text-align` (which you seem to have fixed)
* Interaction with floats (no tests in WPT)
* Interaction with `initial-letter` (no tests in WPT)
* Interaction with intrinsic sizing (not seriously discussed, but imho one of
the most important questions that would have been worth investigating with the
prototype)
See also questions in the 2nd paragraph of
https://groups.google.com/a/chromium.org/g/blink-dev/c/f5eLz6PIXaI/m/a9OGhvaNAAAJ
<https://groups.google.com/a/chromium.org/g/blink-dev/c/f5eLz6PIXaI/m/a9OGhvaNAAAJ>
which seem to have been mostly ignored...
The #8516 you cited above <https://github.com/w3c/csswg-drafts/issues/8516>
has some initial feedback.
I also asked some specific questions inline, so let me quote them in list form
since they seem to be repeatedly overlooked:
* Will this be sufficiently useful without any integration into intrinsic
sizing?
* Does it actually solve the problems authors want it to solve, or is there
something still unsatisfied here?
> Also I will share any feedback we get with the WG and I look forward to
discussing them.
I think it would be useful to know if you have positive feedback from web
developers who have tried to incorporate it into actual designs, or if you
only have feedback that developers are “excited about something like this” but
have not experimented enough to think about important details that might
matter to them (like the interaction with intrinsic sizing).
If spec changes from the discussions, as I stated above, we’re willing to match
the new spec.
You say that, but you're not even willing to match the spec in terms of
shorthanding... :/
~fantasai
--
You received this message because you are subscribed to the Google Groups
"blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-dev+unsubscr...@chromium.org.
To view this discussion on the web visit
https://groups.google.com/a/chromium.org/d/msgid/blink-dev/d66a3d90-f16e-13f2-5ffc-335dda06bd3f%40inkedblade.net.
