Closed Bug 1787871 Opened 2 years ago Closed 2 years ago

Firefox View's "Tab Pickup" card has ineffective -webkit-line-clamp clamping

Categories

(Firefox :: Firefox View, defect, P3)

defect

Tracking

()

RESOLVED FIXED
106 Branch
Tracking Status
firefox106 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Whiteboard: [fidefe-2022-mr1-firefox-view])

Attachments

(5 files, 2 obsolete files)

Attached image screenshot

Not sure about precise STR, but probably:

  • Have your Firefox signed in to a Firefox Account, with another device also connected.
  • Visit about:firefoxview and look at the top left corner.
  • Notice the title of the card. Hopefully it's ellipsized and has some tall characters (e.g. "t" "l") that are after the ellipsis. (If not, use devtools to edit the text there to add some tall characters)

ACTUAL RESULTS:

  • See attached screenshot -- the tops of the characters after the ellipsis were still showing up on the (clipped) next line.
  • It looks like we're styling this element with display:-webkit-box; -webkit-line-clamp: 2 to attempt to prevent that, but it's not working; possibly because this element's being a grid item might be nerfing the -webkit-box display value? Not sure.

EXPECTED RESULTS:
No such partially-visible characters.

It's possible bug 1786147 will help here. That bug is changing how -webkit-line-clamp works, and it might make it Just Work here, or change how it manifests at least.

See Also: → 1786147
Attached file testcase 1
Attachment #9292092 - Attachment is obsolete: true
Attachment #9292092 - Attachment description: testcase 1 (probably) → (ignore me; this version of the testcase doesn't work because its letters aren't tall enough)

Is there an alternative way of doing this that doesn't depend on webkit-line-clamp?

Also, how can this both not be working and yet have the ellipsis showing as in the screenshot? The ellipsis should be based on the line clamping... so how come it only "half" works?

This isn't visible on macOS because the maximum height and clipping makes things work. Is this only reproducible on Linux and/or with non-latin text? (I expect it depends on the details of the font metrics whether the 2.5em height assigned to this thing is "right")

Depends on: 1786147
Flags: needinfo?(dholbert)
See Also: 1786147
Whiteboard: [fidefe-2022-mr1-firefox-view]

(In reply to :Gijs (he/him) from comment #5)

Is there an alternative way of doing this that doesn't depend on webkit-line-clamp?

I'm not aware of one. There is a standard version https://www.w3.org/TR/css-overflow-3/#propdef-line-clamp but that's not implemented anywhere yet.

Also, how can this both not be working and yet have the ellipsis showing as in the screenshot? The ellipsis should be based on the line clamping... so how come it only "half" works?

Hmm, I'd forgotten but that's apparently just how webkit-line-clamp "works", in all browsers. Sigh.

See e.g. https://wpt.live/css/css-overflow/webkit-line-clamp-005.html (which crops off one line) -- if you use devtools to edit the clamp node there to have e.g. height:500px, then you can see the cropped line (after the ellipsis). This happens in all browsers.

This isn't visible on macOS because the maximum height and clipping makes things work. Is this only reproducible on Linux and/or with non-latin text?

So far I've only tested on Linux; not sure about other platforms. It looks like the element in question is using Ubuntu Medium as the font which maybe (?) has particularly-unfortunate font metrics here. (It doesn't require non-latin text; see my attached testcase).

Looking at other usages of webkit-line-clamp (e.g. in cards on the new-tab page), it looks like typically folks avoid this by specifying a generous line-height which is sufficient to push the clipped lines beyond the height and ensure they are clipped. Maybe we should increase the line-height a bit here? (and/or reduce the height?)

Flags: needinfo?(dholbert)

(In reply to Daniel Holbert [:dholbert] from comment #6)

(In reply to :Gijs (he/him) from comment #5)

Also, how can this both not be working and yet have the ellipsis showing as in the screenshot? The ellipsis should be based on the line clamping... so how come it only "half" works?

Hmm, I'd forgotten but that's apparently just how webkit-line-clamp "works", in all browsers. Sigh.

See e.g. https://wpt.live/css/css-overflow/webkit-line-clamp-005.html (which crops off one line) -- if you use devtools to edit the clamp node there to have e.g. height:500px, then you can see the cropped line (after the ellipsis). This happens in all browsers.

Oh, wait, so this isn't actually meant to genuinely crop stuff... you have to do that yourself (no matter grid/webkit-box intricacies)? The only thing it really does is the ellipsis, but then only if you use text-overflow: ellipsis? That makes... no sense!? Like, the layout model clearly knows which line is correct, why not actually cut off the text at that point!? I'm very confused...

It does seem like that is half-arsedly documented on MDN:

In most cases you will also want to set overflow to hidden, otherwise the contents won't be clipped but an ellipsis will still be shown after the specified number of lines.

I mean, unless there's a "some other cases" to the "most cases", it sounds like the overall description of what this does is just wrong? Under what circumstances does it clip the surplus lines?!

This isn't visible on macOS because the maximum height and clipping makes things work. Is this only reproducible on Linux and/or with non-latin text?

So far I've only tested on Linux; not sure about other platforms. It looks like the element in question is using Ubuntu Medium as the font which maybe (?) has particularly-unfortunate font metrics here. (It doesn't require non-latin text; see my attached testcase).

Sorry, I wasn't clear. I meant, it's clearly broken for you on Linux with "boring" ascii latin text, but I imagine you could "break" it on macOS/Windows with characters with more, uh, interesting font metrics (even in default fonts).

Looking at other usages of webkit-line-clamp (e.g. in cards on the new-tab page), it looks like typically folks avoid this by specifying a generous line-height which is sufficient to push the clipped lines beyond the height and ensure they are clipped. Maybe we should increase the line-height a bit here? (and/or reduce the height?)

Yeah, I guess that would help a bit, but it's unfortunate either way.

Flags: needinfo?(dholbert)
Blocks: firefox-view

(In reply to :Gijs (he/him) from comment #7)

Oh, wait, so this isn't actually meant to genuinely crop stuff... you have to do that yourself (no matter grid/webkit-box intricacies)? The only thing it really does is the ellipsis,

Yes; it adds an ellipsis at Line N, where N is the thing you speciifed.

but then only if you use text-overflow: ellipsis?

No, you don't need to use text-overflow:ellipsis (See e.g. https://wpt.live/css/css-overflow/webkit-line-clamp-005.html which doesn't use that, but does get an ellipsis).

In our Matrix discussion, I did mention text-overflow:ellipsis, but that was as an alternative if we did this as a single-line thing.

That makes... no sense!? Like, the layout model clearly knows which line is correct, why not actually cut off the text at that point!? I'm very confused...

-webkit-line-clamp is very much indeed Not A Great Feature.

There are standardized versions, but nobody has implemented them yet; whereas every non-gecko engine is ultimately WebKit-derived and hence they all have this not-great thing; and so that's what websites depend on; so it's what we implemented for now, in Bug 866102.

Nobody loves it, but it's de-facto required and it's there.

Under what circumstances does it clip the surplus lines?!

It looks like the CSSWG/spec-editors who designed the unprefixed version of this property intend for continue: discard (and continue: -webkit-discard) to make the clipped lines be non-rendered. But those don't seem to be implemented anywhere yet.
https://drafts.csswg.org/css-overflow-3/#valdef-continue-discard
https://drafts.csswg.org/css-overflow-3/#valdef-continue--webkit-discard

I'm not sure continue:discard and/or -webkit-discard would be particularly complex to implement as an extension to this existing functionality... To the extent that there's complexity hiding there, it'd come from being robust against hypothetical JS mutation (inserting/deleting text, pushing it into & out of the clipped region); and we could conceivably punt on that complexity and implement a nerfed/good-enough version for use here, and have it enabled only for Firefox frontend code, with the intent of hardening it before exposing it to the web.

Having said that, I'm speculating a bit and would rather not have that hold up this whole Firefox View feature.

Sorry, I wasn't clear. I meant, it's clearly broken for you on Linux with "boring" ascii latin text, but I imagine you could "break" it on macOS/Windows with characters with more, uh, interesting font metrics (even in default fonts).

Yup, that's sort of breakage is surely possible (with interesting characters and/or fonts).

It's similarly possible for interesting characters in the expected-to-render lines (lines 1 and 2) to get awkwardly clipped at the top/left/right/bottom bounary of the overflow:hidden container. (that's probably less confusing to users than phantom fringes-of-letters-in-clipped-lines though)

Maybe we should increase the line-height a bit here? (and/or reduce the height?)
Yeah, I guess that would help a bit

I'd suggest we move forward with that for now...

Flags: needinfo?(dholbert)
Depends on: 1787984
See Also: → 1787984

Actually, a better solution here is to just use a content-based height on this element. The thing that's causing the next line to be visible is the fact that we're making this element taller than the 2 lines that we requested of it.

As long as it has a reasonable line-height (which it does): If we just let this element be as tall as its lines, then I think it should be appropriately-tall to include the descenders, without inadvertently catching pieces of the ascenders from the next line like it's doing right now.

To do that, we would need to:
(1) remove the specified height:2.5em that we have on this element right now
This leaves it with height:auto which then makes it stretch by default (per default align-self which stretches auto heights to fill the grid cell). To avoid that stretching, we need to then
(2) give it align-self: start

Then it will align to the top of its grid cell (just as it does now) and it won't stretch its height to be any taller than the 2 lines of text that we're requesting it to have.

These changes seem to fix the issue if I perform them in devtools.

(I can't test this directly now because my local build is currently syncing just fine but not showing synced tabs on Firefox View; not sure why and it's too late to figure that out at the moment.)

er sorry, I typo'd one of the lines. :) This is the patch I meant to attach. (Still un-tested except as devtools tweaks in my Nightly.)

Attachment #9292203 - Attachment is obsolete: true

(In reply to Daniel Holbert [:dholbert] from comment #11)

Created attachment 9292204 [details]
maybe a fix (just attaching for demonstration)

er sorry, I typo'd one of the lines. :) This is the patch I meant to attach. (Still un-tested except as devtools tweaks in my Nightly.)

Yeah, this seems to work for me, too. I'll attach on phab and get a second/third pair of eyes on it.

Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/bc03949f46f6
fix line clamping in the title of the Firefox View tab pickup hero tab card, r=sclements
Severity: -- → S4
Priority: -- → P3
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 106 Branch

Credit where due etc. :-)

Assignee: gijskruitbosch+bugs → dholbert
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: