Repaint rect does not take into account text-underline-offset

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Jon Lee]

https://codepen.io/legion80/pen/MWQPoRZ

Hover over the two links.

The hover should turn the underline color to red. It does when the decoration is within the bounding box of the text, but does not when it's outside.

[zalan]

It looks like the underline is supposed to be part of the ink overflow but for some reason we don't take it into account when computing it. When ink overflow is inflated with box-shadow, repaint works just fine.

[zalan]

https://jsfiddle.net/7f16hr4d/

[zalan]

Created attachment 460109 [details]
Test reduction

[zalan]

Created attachment 460110 [details]
Test reduction

make it less flaky by adding some non-zero timeout

[Radar WebKit Bug Importer]

<rdar://problem/94686075>

[zalan]

Created attachment 460155 [details]
Patch

[zalan]

(In reply to zalan from comment #6)
> Created attachment 460155 [details]
> Patch
need to come up with some test cases including vertical and flipped modes.

[zalan]

Created attachment 460163 [details]
Patch

[Myles C. Maxfield]

Last time I tried to do this, it caused a large performance regression due to repaint rects intersecting new renderers, which caused more to be rendered than used to be rendered. Are you confident this doesn't cause a performance regression?

[Myles C. Maxfield]

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

[zalan]

(In reply to Myles C. Maxfield from comment #9)
> Last time I tried to do this, it caused a large performance regression due
> to repaint rects intersecting new renderers, which caused more to be
> rendered than used to be rendered. Are you confident this doesn't cause a
> performance regression?
Wasn't that mostly about RenderStyle::changeAffectsVisualOverflow (https://trac.webkit.org/changeset/244277)? None of that has changed here. This is about matching ink overflow produced by legacy line layout (and while this patch certainly has performance implications when certain properties are present, we don't really have a choice but inflate the ink overflow rect as needed).

[zalan]

Comment on attachment 460163 [details]
Patch

repaintrect height went from 12 to 13 (probably because of the +1 to compensate for the integral ceiling in GraphicsContext::computeLineBoundsAndAntialiasingModeForText()
will rebaseline.

[zalan]

Created attachment 460171 [details]
Patch

[EWS]

Committed r295472 (251477@main): <https://commits.webkit.org/251477@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 460171 [details].

[zalan]

Thank you for reporting this bug!

[Darin Adler]

In future can just use “internals” instead of “window.internals” in almost all cases.

[zalan]

(In reply to Darin Adler from comment #16)
> In future can just use “internals” instead of “window.internals” in almost
> all cases.
Thanks, will do!

[zalan]

*** Bug 242570 has been marked as a duplicate of this bug. ***