Bug 192495 – Range.getBoundingClientRect() returns wrong size when started before a br tag.

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: Layout and Rendering

[Gabe Giosia]

Created attachment 356797 [details]
Calculating the height of Range that is adjacent to a break is incorrect in WebKit.

Calculating the height of Range that includes a <br> often returns incorrect sizes. This seems to happen when the start of the range is a node that is a sibling of a <br> that is included in the range.

<div>ABCDEFGHI</div>
range(CD), height = 18
range(CDEFG), height = 18

<div>ABC<br>DEF<br>GHI</div>
range(CD), height = 18 (Should be 34)
range(CDEFG), height = 34 (Should be 50)


WORKAROUND: Don't allow startContainer of a range to be siblings of <br> nodes by wrapping the start text in a span.

<div><span>ABC</span><br>DEF<br>GHI</div>
range(CD), height = 34
range(CDEFG), height = 50

[Gabe Giosia]

Created attachment 363351 [details]
Bypass simpleLineLayout for text that has siblings

[Work in progress] This probably isn't how this should be done because it impacts unrelated parts of the app, but it does fix the issue and I made added a test.

[EWS Watchlist]

Comment on attachment 363351 [details]
Bypass simpleLineLayout for text that has siblings

Attachment 363351 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11333848

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 363355 [details]
Archive of layout-test-results from ews105 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Ryosuke Niwa]

Comment on attachment 363351 [details]
Bypass simpleLineLayout for text that has siblings

In particular, this patch needs to be reviewed first (set r?), and needs a change log.
You can use Tools/Scripts/webkit-patch upload to do both of these things automatically
Please see https://webkit.org/contributing-code/

[Gabe Giosia]

Created attachment 363508 [details]
Bypass simpleLineLayout for text that has siblings

This makes RenderText only use simpleLineLayout when evaluating a single text node. This fixes an issue where Ranges

I looked at refactoring the RunResolver in SimpleLineLayoutResolver.cpp to get simpleLineLayout working with line-breaks, but kept running into situations that didn't make sense to me(I think I'm misunderstanding how RunResolver is supposed to work).

[EWS Watchlist]

Comment on attachment 363508 [details]
Bypass simpleLineLayout for text that has siblings

Attachment 363508 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11362361

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 363509 [details]
Archive of layout-test-results from ews101 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[zalan]

Comment on attachment 363508 [details]
Bypass simpleLineLayout for text that has siblings

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

> Source/WebCore/rendering/RenderText.cpp:1335
> +    if (!is<RenderBlockFlow>(*parent()) || parent()->firstChild() != parent()->lastChild())

We shouldn't need to land changes like this (papering over instead of addressing the actual issue) in normal circumstances. I'd rather have the correct fix instead.

[Gabe Giosia]

I'll dive deeper and parse-out what is going wrong in the simpleLineLayout path.

When using simpleLineLayout the rects for the last text-node are empty because the iterator gets messed up... but the code is hard to follow so I need to spend some more time understanding exactly how it's supposed to be operating.

[Gabe Giosia]

Created attachment 363829 [details]
Fix Range end offsets

Uses local-end offsets rather than using the end-offset within the node. This was mostly working before because contained nodes use maxInt to signify fully contained.

[EWS Watchlist]

Comment on attachment 363829 [details]
Fix Range end offsets

Attachment 363829 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/11407011

New failing tests:
fast/dom/Range/getClientRects.html
fast/dom/Range/simple-line-layout-getclientrects.html

[EWS Watchlist]

Created attachment 363834 [details]
Archive of layout-test-results from ews113 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[Gabe Giosia]

*** This bug has been marked as a duplicate of bug 182181 ***