Bug 262289 - Matched properties cache doesn't account for lh units correctly.
Summary: Matched properties cache doesn't account for lh units correctly.
Status: RESOLVED WORKSFORME
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: BrowserCompat
Depends on:
Blocks:
 
Reported: 2023-09-28 01:45 PDT by Emilio Cobos Álvarez (:emilio)
Modified: 2023-09-29 07:09 PDT (History)
4 users (show)

See Also:

Attachments
rendering in Safari, firefox, chrome (134.22 KB, image/png)
2023-09-28 18:47 PDT, Karl Dubost 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Emilio Cobos Álvarez (:emilio) 2023-09-28 01:45:54 PDT 
What steps will reproduce the problem?

(1) Open https://crisal.io/tmp/lh-sharing.html
(2) See how both blocks are 50px tall, even though the line-height of the second one is 100px.

I caught this while reviewing the Firefox patches to support lh/rlh.

Chromium bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1487480
Comment 1 Karl Dubost 2023-09-28 18:47:00 PDT 
Created attachment 467969 [details]
rendering in Safari, firefox, chrome

Thanks Emilio.
I can not reproduce on STP 179.
Comment 2 Karl Dubost 2023-09-28 18:56:52 PDT 
document.querySelectorAll('div')[2].getBoundingClientRect()

returns 100 in STP 179 and Firefox 120.0a1 (2023-09-28) (64-bit)
and returns 50 in Version 119.0.6036.0 (Official Build) canary (arm64)

So it was probably fixed.
Comment 3 Emilio Cobos Álvarez (:emilio) 2023-09-29 03:28:56 PDT 
I don't see anything from here that deals with line-height, so I'm skeptic that it has been fixed: https://searchfox.org/wubkat/rev/c5cb966bfdbd256be13f3a3c5eafaec9eccb1f0a/Source/WebCore/style/MatchedDeclarationsCache.cpp#61-101
Comment 4 Karl Dubost 2023-09-29 07:09:33 PDT 
Emilio

fwiw the test is not working on Safari 17 released version. 
So this is a recent fix. Let's Autospade

Bug 260199
https://github.com/WebKit/WebKit/commit/b733d192e11b8379df67740799a87cecaa565a85