Bug 205527

Summary: REGRESSION (r212693): getClientRects(), getBoundingClientRect() for range that spans multi-lines differs depending on whether text is selected
Product: WebKit Reporter: Daniel Bates <dbates>
Component: TextAssignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: esprehn+autocc, ews-watchlist, glenn, kondapallykalyan, mmaxfield, pdr, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar, Regression
Version: WebKit Local Build   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=205563
Bug Depends on: 168613    
Bug Blocks: 205563    
Attachments:
Description Flags
Test
none
For the bots
none
For the bots now with Mac-only tests
none
Patch and Mac-specific test
none
Patch and Mac-specific test none

Daniel Bates
Reported 2019-12-20 15:58:43 PST
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.
Attachments
Test (1.49 KB, text/html)
2019-12-20 15:58 PST, Daniel Bates 		no flags
Details
For the bots (2.36 KB, patch)
2019-12-23 11:54 PST, Daniel Bates 		no flags
DetailsFormatted DiffDiff
For the bots now with Mac-only tests (6.21 KB, patch)
2019-12-23 13:30 PST, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Patch and Mac-specific test (9.73 KB, patch)
2019-12-23 14:40 PST, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Patch and Mac-specific test (9.73 KB, patch)
2019-12-23 14:43 PST, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2019-12-20 15:58:54 PST
<rdar://problem/58128278>
Daniel Bates
Comment 2 2019-12-23 09:10:52 PST
Regressed with fix for bug #168613.
Daniel Bates
Comment 3 2019-12-23 11:50:22 PST
I'm going to take a stab at this.
Daniel Bates
Comment 4 2019-12-23 11:54:34 PST
Created attachment 386346 [details] For the bots
Daniel Bates
Comment 5 2019-12-23 12:50:20 PST
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
Comment 6 2019-12-23 13:30:22 PST
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
Comment 7 2019-12-23 13:31:36 PST
Forgot to mention test has expected failure results due to bug #205563
Daniel Bates
Comment 8 2019-12-23 14:40:06 PST
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
Comment 9 2019-12-23 14:43:28 PST
Created attachment 386357 [details] Patch and Mac-specific test
Daniel Bates
Comment 10 2019-12-23 15:40:12 PST
Thank you Zalan!!!
Daniel Bates
Comment 11 2019-12-23 15:40:23 PST
Alan!
Daniel Bates
Comment 12 2019-12-23 15:49:17 PST
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 13 2019-12-23 16:32:10 PST
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
Comment 14 2019-12-23 16:32:12 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.