Multi-line text clipped to its background is painted in the wrong place

• Component: Layout and Rendering
• Status: NEW ---

[Myles C. Maxfield]

Multi-line text clipped to its background is painted in the wrong place

[Myles C. Maxfield]

Created attachment 438317 [details]
Patch

[Myles C. Maxfield]

<rdar://problem/59928689>

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Myles C. Maxfield]

Created attachment 438405 [details]
Patch

[zalan]

Comment on attachment 438405 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=438405&action=review

> Source/WebCore/rendering/LegacyInlineFlowBox.cpp:1215
> -            for (auto* curr = prevLineBox(); curr; curr = curr->prevLineBox())
> +            for (auto* curr = previousOnLine(); curr; curr = curr->previousOnLine())

I don't really know how this fill layer painting is supposed to work but according to the comment a few lines above, we are trying to form a long "strip" here. This strip includes the entire inline box (<span> in this case) with all its fragments spanning across multiple lines, if applicable. e.g <b>first line</b><span>still first line<br>second line</span>, would form 3 lines boxes across the 2 lines, one for the <b> and 2 for the <span>. When we are on the second line, the loop with prevLineBox() would collect the second line box from the first line (which belongs to the same inline box, <span> in this case). Then we offset the stripX with the logicalOffsetOnLine value potentially producing a negative starting point. The logical right of this strip should still fully include the current line box though. Not sure why we do this, but that's what the comment describes above. Now with your change we stop collecting the fragments from the previous line(s) (though interestingly we still do it for the next line(s) in the subsequent loop). If that's the desired behavior the comment should be updated accordingly. However previousOnLine() crosses the inline box boundary (as it walks all the boxes on the line) so our offset again would be incorrect if there was a previous content on the line (see the example above). I haven't tried this but it looks to be working only when the inline box starts at the logical left of the line.