1786147 - -webkit-line-clamp not working on Play Store pages

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Thomas Wisniewski [:twisniewski]]

Unlike in webkit/blink, the -webkit-line-clamp:6 on the app description text at the Play Store is not working, leading to strange overlapping text and other layout quirks. I'm not sure why this is the case. I wonder if this is related to https://bugzilla.mozilla.org/show_bug.cgi?id=1720119, but I'm not sure.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Can you elaborate? I see the same behavior in WebKit, for the record. So this is a difference with Blink, but not with WebKit, afaict.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Test-case.

Afaict this is expected from our current -webkit-line-clamp implementation, which matches what WebKit and Blink implemented at the time, at least.

Basically, we clamp each flex item individually. Since there's no wrapper block across all the description text, that's clamping each text-node individually, causing the weird effect.

Wrapping all the contents of class="clamp" in a single flex item works.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

It seems Blink now special-cases "old flex box with line-clamp" and just creates a single block: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/layout/layout_object.cc;l=356;drc=312b74e385e6aba98ab31fd911238c0dc16b396c

Which explains this difference.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1786147 - Move line-clamp out of mako and do some adjacent clean-up. r=#style,#layout

No behavior change, but simplifies the following patch.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1786147 - Make -webkit-line-clamp create a block container in the appropriate situations. r=dholbert

This is a hack, similar to Chromium's (comment 2) except at computed-value
rather than used-value time, because it's both simpler to reason about and
prevents lying in the computed style.

This fixes the relevant test-case, and matches closer what Chromium does. I
need to update some tests so take it as a request-for-feedback for now.

Depends on D155180

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1786147 - Remove layout.css.webkit-line-clamp.enabled, and add a new pref to apply line-clamp to all BFCs. r=dholbert

It's always true, so remove it. Add another pref to allow -webkit-line-clamp to
work on all blocks rather than just legacy -webkit-boxes, which seems something
we should try to look into, eventually.

Depends on D155181

Comment 7

[Thomas Wisniewski [:twisniewski]]

Ah, I didn't realize that the webkit behavior differed here, actually. I should have checked that, thanks for finding out what happened. This seems like something we should let Apple know about as well.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

FYI Karl, since Safari behaves like (current) Firefox.

Comment 9

[Karl Dubost💡 :karlcow]

Thanks Emilio, Opened https://bugs.webkit.org/show_bug.cgi?id=244286
The commit for when it was added to blink is:
https://github.com/chromium/chromium/commit/383112a

unfortunately there is no associated bug number.

Comment 10

[Ian Kilpatrick [:iank]]

Hey Emilio - yeah we actually did this work in response to Firefox implementing the -webkit-line-clamp behaviour. We did this to match the spec for line-clamp (https://drafts.csswg.org/css-overflow-3/#line-clamp) so that we could transparently switch over our -webkit-line-clamp implementation when we implement the more complex line-clamp behaviour. We discussed our intentions with various folks (I think heycam from FF at the time).

This work also had the side-effect of matching Firefox for more complex cases like:
https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=10632
(effectively the -webkit-line-clamp "counts" within the formatting-context, and doesn't descent into new formatting contexts).

The previous behaviour was slightly bananas and doesn't really match developer expectations, e.g. https://twitter.com/davatron5000/status/1428751058822647810

We didn't find anyone relying on "flex" behaviours (e.g. flexing behaviour, alignment, etc).
There will be about ~3 WPT tests that will need to be updated.

Let me know if you have any questions.

Ian

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d7a22c187b43
Move line-clamp out of mako and do some adjacent clean-up. r=boris

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f257ce89025
Fix typo to avoid devtools property-db failures.

Comment 13

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/d7a22c187b43
https://hg.mozilla.org/mozilla-central/rev/7f257ce89025

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1a797651f1e5
Make -webkit-line-clamp create a block container in the appropriate situations. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/b18f0a025ff1
Remove layout.css.webkit-line-clamp.enabled, and add a new pref to apply line-clamp to all BFCs. r=dholbert

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/35826 for changes under testing/web-platform/tests

Comment 16

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/1a797651f1e5
https://hg.mozilla.org/mozilla-central/rev/b18f0a025ff1

Comment 17

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot