Bug 282444 – REGRESSION(272476@main): No ligatures across before/after and actual content

• Status: NEW
• Component: Layout and Rendering

[alan]

Created attachment 473106 [details]
Test case

see attached test case

[alan]

B---------- --      BODY RenderBody at (8,8) size 1384x791 renderer (0x113005cd0) layout box (0x113007a50) node (0x113006d40) layout id->[8]
            --        line at (0.00,0.00) size (1384.00x460.00) baseline (360.00) enclosing top (0.00) bottom (460.00)
            --          Root inline box at (0.00,0.00) size (811.13x460.00)
            --            Inline box at (0.00,0.00) size (244.34x460.00) renderer->(0x113007200)
            --            Inline box at (566.80,0.00) size (244.34x460.00) renderer->(0x1130076b0)
            --              Inline box at (700.00,0.00) size (111.13x460.00) renderer->(0x113007940)
            --          Run(s):
            --            Text at (0.00,0.00) size 133.20x460.00 run(0, 1) renderer->(0x113007310)
            --            Text at (133.20,0.00) size 111.13x460.00 run(0, 1) renderer->(0x113007400)
            --            Text at (244.34,0.00) size 322.46x460.00 run(0, 3) renderer->(0x1130075e0)
            --            Text at (566.80,0.00) size 133.20x460.00 run(0, 1) renderer->(0x1130077c0)
            --            Text at (700.00,0.00) size 111.13x460.00 run(0, 1) renderer->(0x1130074f0)

These text boxes need to be merged so that painting could apply ligatures -it's a pity though that such fragmented layout output affects painting in this way. painting should be a tiny bit smarter.

[Radar WebKit Bug Importer]

<rdar://problem/139079271>

[Lea Verou]

Please note that this is a regression. I've tested using this codepen https://codepen.io/leaverou/pen/OJKZKyV and at least up to Safari 17.3 the issue is not present, whereas I can reproduce in Safari 17.5 and later.

[alan]

(In reply to Lea Verou from comment #3)
> Please note that this is a regression. I've tested using this codepen
> https://codepen.io/leaverou/pen/OJKZKyV and at least up to Safari 17.3 the
> issue is not present, whereas I can reproduce in Safari 17.5 and later.
👍 thank you!

[alan]

I am somewhat surprised that the last one ever worked as I don't remember we ever merged content from individual renderers, while the middle 'fi' seems to be a tree updater/builder regression as I am seeing 2 renderers constructed for 'content: "f" "i"'.

I----G----- --            <pseudo> RenderInline (::before) renderer (0x13d027810) layout box (0x13d0251a0) node (0x13d027760) layout id->[3]
I---YG----- --              RenderText (::before) renderer (0x13d027920) layout box (0x13d025020) length->(1) "f" layout id->[n/a]
I---YG----- --              RenderText (::before) renderer (0x13d027a10) layout box (0x13d0250e0) length->(1) "i" layout id->[n/a]

[Lea Verou]

Apologies,I should have clarified. It was the first one that used to work (content: “f” “i”;) and now doesn’t!

[alan]

(In reply to Lea Verou from comment #6)
> Apologies,I should have clarified. It was the first one that used to work
> (content: “f” “i”;) and now doesn’t!
yeah, ok, makes sense. Thank you for the clarification!

[Antti Koivisto]

Looks like a regression from https://commits.webkit.org/272476@main

[Tim Nguyen (:ntim)]

Pull request: https://github.com/WebKit/WebKit/pull/36151

[Tim Nguyen (:ntim)]

(In reply to Antti Koivisto from comment #8)
> Looks like a regression from https://commits.webkit.org/272476@main

My PR restores the old behavior regarding generating text runs without regressing serialization: https://github.com/WebKit/WebKit/pull/36151 , but I'm not convinced we should land it given it doesn't fix the other cases Lea brought up, and just makes the code less readable.

Feel free to land if you feel strongly however.

[Lea Verou]

The other cases are not regressions though, so I'd argue they are lower priority.

[Tim Nguyen (:ntim)]

Sending to Alan since he has a better fix that would cover all the cases Lea reported.