Bug 205527 – REGRESSION (r212693): getClientRects(), getBoundingClientRect() for range that spans multi-lines differs depending on whether text is selected

• Status: RESOLVED
• Resolution: FIXED
• Component: Text

[Daniel Bates]

Created attachment 386273 [details]
Test

Steps to reproduce:

1. Open the attached test.

The results before selection and after selection should be identical! They are not.

Maybe this bug should be split into two: one for getClientRects() and one for getBoundingClientRect()? Filing one bug for now as I think the root cause will address both issues also since getBoundingClientRect() is spec'ed to use the result of getClientRects().

I suspect the difference is due to whether we are using simple line layout (the "before selection" result) and line box layout (the "after selection case"). Simple line layout does not appears to return a rect when the selection is at the boundary. In the test case this is the box that represents the " " at the end of the first line, which also happens to coincide with where the text wraps. Simple line layout does not account for this and simply returns the rects from the second line! Line box layout does account for this. I made sure it did as part of the fix for bug #160786.

I suspect ^^^ is also the reason why getBoundingClientRect() returns differing results.

[Radar WebKit Bug Importer]

<rdar://problem/58128278>

[Daniel Bates]

Regressed with fix for bug #168613.

[Daniel Bates]

I'm going to take a stab at this.

[Daniel Bates]

Created attachment 386346 [details]
For the bots

[Daniel Bates]

Comment on attachment 386346 [details]
For the bots

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

> Source/WebCore/rendering/SimpleLineLayoutResolver.cpp:-308
> -    // Special case empty ranges that start at the edge of the run. Apparently normal line layout include those.
> -    if (localEnd == startOffset && (*it).start() == localEnd)
> -        return { rangeBegin, ++it };

Can't delete this.

[Daniel Bates]

Created attachment 386351 [details]
For the bots now with Mac-only tests

Added likely Mac-specific tests...maybe iOS will match. Let us see...

[Daniel Bates]

Forgot to mention test has expected failure results due to bug #205563

[Daniel Bates]

Created attachment 386356 [details]
Patch and Mac-specific test

For now add a Mac-specific test. I will look to come up with a more general purpose test. One way to do this is to use the Ahem font, but it is a bit tricky as I need to figure out the right Web API to ensure that test waits until it has loaded. The Mac-specific test is still useful because this same test can be used for bug #205563. Using Ahem actually masks bug #205563 because Ahem glyphs are too square that it does not allow testing of line layout rouding. Right now, I see a future of having both an Ahem-based version of this test and a Mac-specific one. Maybe drawing borders around the client/border rects could be a way to write a 100% platform-independent, text-metric agnostic test. I will think about it.

[Daniel Bates]

Created attachment 386357 [details]
Patch and Mac-specific test

[Daniel Bates]

Thank you Zalan!!!

[Daniel Bates]

Alan!

[Daniel Bates]

Comment on attachment 386357 [details]
Patch and Mac-specific test

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

> LayoutTests/fast/dom/Range/mac/getClientRects-and-getBoundingClientRect-before-and-after-selection.html:17
> +<pre id="console"></pre>

Oops! That's stray markup from the original test. Will remove before landing.

[Daniel Bates]

Comment on attachment 386357 [details]
Patch and Mac-specific test

Clearing flags on attachment: 386357

Committed r253893: <https://trac.webkit.org/changeset/253893>

[Daniel Bates]

All reviewed patches have been landed.  Closing bug.