LGTM2, it sounds like the compat risk described in https://github.com/w3c/csswg-drafts/issues/8398 prevents making the shorthand/longhand relationships exactly per spec, but that we're taking the same approach here as for baseline-source.
On Thu, Mar 16, 2023 at 6:05 PM Chris Harrelson <chris...@chromium.org> wrote: > LGTM1 > > On Mon, Mar 13, 2023 at 1:02 AM Koji Ishii <ko...@chromium.org> wrote: > >> > This intent to ship just says that you will ship 'text-wrap'. It >>> doesn't say >>> > that you will also implement 'white-space-collapse' (the other >>> longhand of >>> > 'white-space') or the shorthanding relationship between 'text-wrap' and >>> > 'white-space'. I therefore assume you're shipping 'text-wrap' as an >>> > independent property, and this is not going to be spec-compatible. >> >> >>> Koji will need to speak to this. >> >> >> We have implemented a similar approach as the >> vertical-align/baseline-source case >> <https://groups.google.com/a/chromium.org/g/blink-dev/c/gLNyu0lTRS0/m/GqVBzSsbAAAJ> >> so that setting the `white-space` resets the `text-wrap: wrap`. The current >> impl passes this cascading test >> <https://github.com/web-platform-tests/wpt/blob/69982c63f5/css/css-text/white-space/text-wrap-white-space-001.html> >> . >> > > This mitigation makes sense to me and I think it's sufficient. In the case > of the intent you're linking to here, vertical-align is specified to be a > shorthand of baseline-source, and after feedback about the issue on that > intent thread, Ian implemented > <https://chromium-review.googlesource.com/c/chromium/src/+/4189506> a > mapping from existing supported uses of vertical-align to baseline-source. > And Koji implemented the same approach for white-space, which is specified > to be a shorthand of text-wrap. > > Just as with the other intent, there are other possible compat risks; in > addition to some mentioned in the other intent thread, there is this > CSSWG issue <https://github.com/w3c/csswg-drafts/issues/8398> discussing > it. But I don't think we have any information leading us to conclude that > the risk analysis we performed for baseline-source differs from the one > we're discussing here, so I don't consider these concerns blocking shipping > text-wrap. > > Looks like the tests are in >>> https://wpt.fyi/results/css/css-text/white-space?label=master&label=experimental&aligned&view=subtest&q=balance. >>> There's one failing test there, do you know why that is? >> >> >> Thanks for pointing this out. The test passes in Chromium repo, I'll >> investigate why it fails in wpt. >> >> And overall, are you happy with the test coverage here, does it cover >>> most important corner cases? >> >> >> I'm keeping eyes on incoming issues and feedback. It's a pleasant >> surprise that more web devs than expected are excited with this feature, >> and gave us great feedback such as this test results >> <https://front-end.social/@kizu/109950116535590284>. Paragraph-level >> line breaking is a new area for browsers, there should be more rooms to >> improve further and I'm looking into them, but I think it's at a good >> quality to ship, and we can expect more feedback for the next stage. >> >> On Thu, Mar 9, 2023 at 1:50 AM Philip Jägenstedt <foo...@chromium.org> >> wrote: >> >>> Hi Koji, >>> >>> Looks like the tests are in >>> https://wpt.fyi/results/css/css-text/white-space?label=master&label=experimental&aligned&view=subtest&q=balance. >>> There's one failing test there, do you know why that is? And overall, are >>> you happy with the test coverage here, does it cover most important corner >>> cases? >>> >>> Best regards, >>> Philip >>> >>> On Sun, Mar 5, 2023 at 9:26 PM Tab Atkins Jr. <jackalm...@gmail.com> >>> wrote: >>> >>>> On Thu, Mar 2, 2023 at 6:59 PM fantasai <fantasai.li...@inkedblade.net> >>>> wrote: >>>> > On 3/2/23 17:48, Tab Atkins Jr. wrote: >>>> > > If you think there are specific issues that need to be fixed (that >>>> > > don't equally need to be fixed for text-wrap:wrap), could you list >>>> > > them? >>>> > >>>> > The 'text-wrap' property is a longhand of 'white-space'. If you don't >>>> > implement it as such, the way that 'text-wrap' and 'white-space' >>>> declarations >>>> > override each other will change if/when you do. >>>> > >>>> > This intent to ship just says that you will ship 'text-wrap'. It >>>> doesn't say >>>> > that you will also implement 'white-space-collapse' (the other >>>> longhand of >>>> > 'white-space') or the shorthanding relationship between 'text-wrap' >>>> and >>>> > 'white-space'. I therefore assume you're shipping 'text-wrap' as an >>>> > independent property, and this is not going to be spec-compatible. >>>> >>>> Koji will need to speak to this. >>>> >>>> > > We probably do want some type of control for orphan words, but it's >>>> an >>>> > > unrelated problem; *all* of the text-wrap options can want it. >>>> Likely >>>> > > it belongs in its own property. >>>> > >>>> > How is (a hypothetical) 'last-line-length: 100%' different from >>>> 'text-wrap: >>>> > balance'? If they're the same, does it make sense to have 'text-wrap: >>>> balance'? >>>> >>>> They're quite different. A balanced paragraph can very easily have the >>>> last line *not* fill 100% of the line box, because being balanced >>>> actually means averaging an 80% fill, for example; there's no way to >>>> predict what the ideal fill is in a whole-para balance. There's also >>>> perf concerns - a last-line control might very reasonably only try to >>>> adjust the last several lines, rather than the entire paragraph, so >>>> that it's generally usable on all text rather than just text short >>>> enough to be efficiently whole-para balanceable. >>>> >>>> ~TJ >>>> >>>> -- >>>> 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/CAAWBYDAmKU3T-M-yyM5SywbSVj2iTUcdqWXCNSdSV6Xco3TGdw%40mail.gmail.com >>>> . >>>> >>> -- >>> 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/CAARdPYdZTqRA5x2qpsBZq00Dzvv%3DW-y9cGT4UKxVZXKExfs0HA%40mail.gmail.com >>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAARdPYdZTqRA5x2qpsBZq00Dzvv%3DW-y9cGT4UKxVZXKExfs0HA%40mail.gmail.com?utm_medium=email&utm_source=footer> >>> . >>> >> -- >> 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/CAHe_1dJ8QeZFYbCQHUNdpPFZG2Sx1%3DmTLco-CABCV6u1ogF%3D6A%40mail.gmail.com >> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAHe_1dJ8QeZFYbCQHUNdpPFZG2Sx1%3DmTLco-CABCV6u1ogF%3D6A%40mail.gmail.com?utm_medium=email&utm_source=footer> >> . >> > -- 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/CAARdPYebYUBWG0j8RN22GvHaRegjDCxQzvSwdo96PthAm9f%3DBg%40mail.gmail.com.
