Closed Bug 1786147 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: CSS Parsing and Computation, defect)

defect

Tracking

()

RESOLVED FIXED
106 Branch
Tracking Status
firefox106 --- fixed

People

(Reporter: twisniewski, Assigned: emilio)

References

(Blocks 1 open bug, Regressed 1 open bug, )

Details

Attachments

(4 files)

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.

Summary: -webkit-line-clamp does not work with -webkit-box-orient:vertical → -webkit-line-clamp not working on Play Store pages

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.

Flags: needinfo?(twisniewski)
Attached file 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.

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.

No behavior change, but simplifies the following patch.

Assignee: nobody → emilio
Status: NEW → ASSIGNED

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

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

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.

Flags: needinfo?(twisniewski)
Severity: -- → S3

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

Flags: needinfo?(karl+moz)

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.

Flags: needinfo?(karl+moz)

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

See Also: → 1787871
Blocks: 1787871
See Also: 1787871
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d7a22c187b43
Move line-clamp out of mako and do some adjacent clean-up. r=boris
Keywords: leave-open
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/7f257ce89025
Fix typo to avoid devtools property-db failures.
Attachment #9290824 - Attachment description: Bug 1786147 - Remove layout.css.webkit-line-clamp.enabled. r=dholbert → Bug 1786147 - Remove layout.css.webkit-line-clamp.enabled, and add a new pref to apply line-clamp to all BFCs. r=dholbert
Pushed by ealvarez@mozilla.com:
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
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/35826 for changes under testing/web-platform/tests
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Upstream PR merged by moz-wptsync-bot
Regressions: 1789781
Regressions: 1790882
Flags: in-testsuite+
Target Milestone: --- → 106 Branch
Regressions: 1808814
You need to log in before you can comment on or make changes to this bug.