WebKit / Speedometer

An open source repository for the Speedometer benchmark
Other
614 stars 73 forks source link

In news site, `.truncate-multiline` uses deprecated `-webkit-box` #318

Closed rniwa closed 1 year ago

rniwa commented 1 year ago

Right now, both news site tests use the following CSS rule:

.truncate-multiline {
    display: -webkit-box !important;
    -webkit-line-clamp: 2;
    -webkit-box-orient: vertical;
    width: 100%;
    overflow:hidden
}

This would result in the deprecated flexbox layout to kick in. I don't think that's appropriate in a supposedly cross-browser benchmark.

rniwa commented 1 year ago

cc @flashdesignory

flashdesignory commented 1 year ago

looking at other workloads, there are many uses of -webkit specific css rules. Are we removing all of those too?

https://github.com/search?q=repo%3AWebKit%2FSpeedometer%20-webkit&type=code

or is it just the display: -webkit-box rule that's problematic?

rniwa commented 1 year ago

Some of those are fine because they also specify the non-prefixed versions but ones that only have effect in WebKit/Blink should probably be removed.

rniwa commented 1 year ago

This deprecated flexbox thing is particularly bad because this stuff is known to be buggy & slow.

flashdesignory commented 1 year ago

It is used in the wild... here are two examples I found quickly:

Screenshot 2023-10-02 at 7 00 31 PM Screenshot 2023-10-02 at 7 00 42 PM
rniwa commented 1 year ago

It is used in the wild... here are two examples I found quickly:

I'm sure it's used in wild somewhere. That doesn't mean we should be using it in Speedometer content.

bgrins commented 1 year ago

This would result in the deprecated flexbox layout to kick in. I don't think that's appropriate in a supposedly cross-browser benchmark.

...

but ones that only have effect in WebKit/Blink should probably be removed.

This technique works in Gecko as well: we support -webkit-box and associated properties, so if you open the page in Firefox it has the same multiline clamp + ellipsis UX.

There's a separate question about UX itself and the particular techniques for doing it, but I'm not persuaded this should be removed as a cross browser concern.

rniwa commented 1 year ago

This would result in the deprecated flexbox layout to kick in. I don't think that's appropriate in a supposedly cross-browser benchmark.

...

but ones that only have effect in WebKit/Blink should probably be removed.

This technique works in Gecko as well: we support -webkit-box and associated properties, so if you open the page in Firefox it has the same multiline clamp + ellipsis UX.

Oh, I wasn't aware of that. In that case, we can keep it around.