Bug 205563

Summary: Precision of getClientRects(), getBoundingClientRect() differs depending whether simple line layout or line box layout is used
Product: WebKit Reporter: Daniel Bates <dbates>
Component: TextAssignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, ap, esprehn+autocc, ews-watchlist, glenn, kondapallykalyan, mmaxfield, pdr, tsavell, webkit-bot-watchers-bugzilla, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=205527
https://bugs.webkit.org/show_bug.cgi?id=205889
Bug Depends on: 205527    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
To land none

Daniel Bates
Reported 2019-12-23 11:32:42 PST
While looking into bug #205527, I noticed that the rects returned by getClientRects(), getBoundingClientRect() differ in precision depending on whether they were computed using the simple line layout or line box layout code. Steps to reproduce: 1. Open the test case attached to bug #205527: attachment #386273 [details]. Compare the rectangle returned under "getClientRects: Before selection:": <-- simple line layout {"x":8,"y":26,"width":28.453125,"height":18,"top":26,"right":36.453125,"bottom":44,"left":8} to the **second** rectangle returned under "getClientRects: After selection:": <-- line box layout {"x":8,"y":26,"width":29,"height":18,"top":26,"right":37,"bottom":44,"left":8} ^^^ The width using simple line layout is 28.453125, but is 29 using line box layout.
Attachments
Patch (8.88 KB, patch)
2019-12-23 14:50 PST, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Patch (8.88 KB, patch)
2019-12-23 14:53 PST, Daniel Bates 		no flags
DetailsFormatted DiffDiff
To land (20.83 KB, patch)
2020-01-06 10:58 PST, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2019-12-23 11:33:02 PST
<rdar://problem/58165528>
Daniel Bates
Comment 2 2019-12-23 14:50:39 PST
Created attachment 386358 [details] Patch
Daniel Bates
Comment 3 2019-12-23 14:51:21 PST
This patch will fail to apply until bug #205527 is fixed.
Daniel Bates
Comment 4 2019-12-23 14:53:35 PST
Created attachment 386360 [details] Patch
Daniel Bates
Comment 5 2019-12-23 15:40:39 PST
Thanks Alan!
Daniel Bates
Comment 6 2019-12-23 18:05:24 PST
Test failures are actually progressions: both the simple line layout and line box layout code paths now return the same rect. The fast/dom/Range/getClientRects.html failure also reveals another issue: the fix for bug #172057 was not enough. More code changes are needed to solve bug #172057 if we still are interested in doing so.
Daniel Bates
Comment 7 2020-01-06 10:58:57 PST
Created attachment 386864 [details] To land
Daniel Bates
Comment 8 2020-01-06 15:08:54 PST
Comment on attachment 386864 [details] To land Clearing flags on attachment: 386864 Committed r254091: <https://trac.webkit.org/changeset/254091>
Daniel Bates
Comment 9 2020-01-06 15:08:56 PST
All reviewed patches have been landed. Closing bug.
Truitt Savell
Comment 10 2020-01-07 12:10:19 PST
Looks like the changes in https://trac.webkit.org/changeset/254091/webkit broke fast/frames/iframe-translucent-background.html tracking this in https://bugs.webkit.org/show_bug.cgi?id=205879
Daniel Bates
Comment 11 2020-01-07 15:05:53 PST
Comment on attachment 386864 [details] To land View in context: https://bugs.webkit.org/attachment.cgi?id=386864&action=review > Source/WebCore/rendering/InlineTextBox.cpp:205 > + auto logicalWidth = snappedSelectionRect.width(); This is wrong. Data type must be LayoutUnit or integer truncation can occur if second branch is taken. Fixed in <https://trac.webkit.org/changeset/254160/webkit>. See bug #205889 for a way to fix this problem to prevent the same mistake in the future.
Daniel Bates
Comment 12 2020-01-07 16:10:28 PST
More layout test fixes landed in <https://trac.webkit.org/changeset/254167>
Aakash Jain
Comment 13 2020-01-08 09:52:22 PST
(In reply to Daniel Bates from comment #11) > Fixed in <https://trac.webkit.org/changeset/254160/webkit>. This broke 6 API tests. Tracked in https://bugs.webkit.org/show_bug.cgi?id=205935
Note You need to log in before you can comment on or make changes to this bug.