Bug 211351 - lh / rlh units don't work properly for some elements.
Summary: lh / rlh units don't work properly for some elements.
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 211356
Blocks: 195180
  Show dependency treegraph
 
Reported: 2020-05-03 06:45 PDT by Emilio Cobos Álvarez (:emilio)
Modified: 2020-05-03 17:45 PDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Emilio Cobos Álvarez (:emilio) 2020-05-03 06:45:13 PDT
There's a bunch of setLineHeight calls in RenderTheme / StyleAdjuster that happen when all other CSS values have already been computed.

So the lh units will refer to a line-height that's fundamentally wrong.

I filed https://github.com/w3c/csswg-drafts/issues/3257 on that a while ago but it seems WebKit has implemented the broken behavior :(
Comment 1 Emilio Cobos Álvarez (:emilio) 2020-05-03 11:52:38 PDT
Ah, that's a MainThreadNeverDestroyed, introduced by:

  https://bugs.webkit.org/show_bug.cgi?id=211264

So this is a regression from that change.
Comment 2 Yusuke Suzuki 2020-05-03 15:16:19 PDT
(In reply to Emilio Cobos Álvarez (:emilio) from comment #1)
> Ah, that's a MainThreadNeverDestroyed, introduced by:
> 
>   https://bugs.webkit.org/show_bug.cgi?id=211264
> 
> So this is a regression from that change.

Could you upload the crash trace of this? If non main thread is touching `const AtomString` generated by `MainThreadNeverDestroyed<>`, this potentially means that `MainThreadNeverDestroyed<>` finds the real bug.
Comment 3 Yusuke Suzuki 2020-05-03 15:18:35 PDT
(In reply to Yusuke Suzuki from comment #2)
> (In reply to Emilio Cobos Álvarez (:emilio) from comment #1)
> > Ah, that's a MainThreadNeverDestroyed, introduced by:
> > 
> >   https://bugs.webkit.org/show_bug.cgi?id=211264
> > 
> > So this is a regression from that change.
> 
> Could you upload the crash trace of this? If non main thread is touching
> `const AtomString` generated by `MainThreadNeverDestroyed<>`, this
> potentially means that `MainThreadNeverDestroyed<>` finds the real bug.

OK, found that, thanks :)
Comment 4 Yusuke Suzuki 2020-05-03 16:13:51 PDT
(In reply to Yusuke Suzuki from comment #3)
> (In reply to Yusuke Suzuki from comment #2)
> > (In reply to Emilio Cobos Álvarez (:emilio) from comment #1)
> > > Ah, that's a MainThreadNeverDestroyed, introduced by:
> > > 
> > >   https://bugs.webkit.org/show_bug.cgi?id=211264
> > > 
> > > So this is a regression from that change.
> > 
> > Could you upload the crash trace of this? If non main thread is touching
> > `const AtomString` generated by `MainThreadNeverDestroyed<>`, this
> > potentially means that `MainThreadNeverDestroyed<>` finds the real bug.
> 
> OK, found that, thanks :)

Looks like that patch (adding new assertions) found a GTK specific bug, which cannot be caught due to lack of GTK Debug EWS.
Comment 5 Emilio Cobos Álvarez (:emilio) 2020-05-03 16:46:35 PDT
Yes, sorry, but I pasted comment 1 in the wrong bug, should've been in bug 211355, that's why I tagged it as off-topic.
Comment 6 Yusuke Suzuki 2020-05-03 17:45:53 PDT
(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)
> Yes, sorry, but I pasted comment 1 in the wrong bug, should've been in bug
> 211355, that's why I tagged it as off-topic.

np! Thanks for reporting :D