First character in each word-wrapped line has incorrect character rect when requested range spans multiple lines

[Daniel Bates]

Consider a page with the following markup:

[[
<style>@font-face { font-family: Ahem; src: url(Ahem.ttf); } body { margin: 0; } * { font: 25px/1 Ahem; -webkit-text-size-adjust: 100%; }</style>
<meta name='viewport' content='width=980, initial-scale=1.0'>
<textarea id='test' style='width: 26em; padding: 0 /* Overrides default user-agent stylesheet */;'>The quick brown fox jumps over the lazy dog.</textarea>
<script>
test.focus(); test.setSelectionRange(25, 25)
</script>
]]

(^^^ Assumes the Ahem font is available)

Then requesting the a document context for rects returns an incorrect character rect for the space/line break after "jumps".

[Daniel Bates]

<rdar://problem/56884325>

[Daniel Bates]

Created attachment 386911 [details]
For Bots

[Daniel Bates]

Created attachment 386989 [details]
Patch and unit test

[Daniel Bates]

Comment on attachment 386989 [details]
Patch and unit test

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

> Source/WebCore/dom/Range.cpp:1152
> +    absoluteTextRects(rects, useSelectionHeight, inFixed, { BoundingRectBehavior::IgnoreEmptyTextSelections });

I patched here because I was kinda thinking this is the behavior we want for all IPI and SPI: it makes us match NSLayoutManager behavior. But it's enough to fix just my particular bug by having <https://trac.webkit.org/browser/trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm?rev=254070#L4074> pass BoundingRectBehavior::IgnoreEmptyTextSelections. That is, Range::absoluteBoundingBox() could take an optional BoundingRectBehavior argument, default such that we do what we do now and just have WebPageIOS.mm's code pass BoundingRectBehavior::IgnoreEmptyTextSelections. Thoughts?

[Daniel Bates]

Created attachment 386996 [details]
Addendum - keep current behavior of Range::absoluteBoundingBox, but fix up request document context code path

[Daniel Bates]

(In reply to Daniel Bates from comment #5)
> Created attachment 386996 [details]
> Addendum - keep current behavior of Range::absoluteBoundingBox, but fix up
> request document context code path

I can merge ^^^ into the patch if it would help make it easier to review this change.

[Daniel Bates]

Created attachment 387011 [details]
To land

[Daniel Bates]

(In reply to Daniel Bates from comment #6)
> (In reply to Daniel Bates from comment #5)
> > Created attachment 386996 [details]
> > Addendum - keep current behavior of Range::absoluteBoundingBox, but fix up
> > request document context code path
> 
> I can merge ^^^ into the patch if it would help make it easier to review
> this change.

Merge this in to the updated patch!

[Daniel Bates]

Comment on attachment 387011 [details]
To land

Clearing flags on attachment: 387011

Committed r254144: <https://trac.webkit.org/changeset/254144>

[Daniel Bates]

All reviewed patches have been landed.  Closing bug.

[Aakash Jain]

This seems to have broken the iOS build. Tracked in https://bugs.webkit.org/show_bug.cgi?id=205883

Relevant error:
WebPageIOS.mm:4064:73: error: use of undeclared identifier 'BoundingRectBehavior'

[Ryan Haddad]

Reverted r254144 for reason:

Broke the iOS build.

Committed r254151: <https://trac.webkit.org/changeset/254151>

[Daniel Bates]

Created attachment 387024 [details]
To land

[Daniel Bates]

Comment on attachment 387024 [details]
To land

Clearing flags on attachment: 387024

Committed r254153: <https://trac.webkit.org/changeset/254153>

[Daniel Bates]

All reviewed patches have been landed.  Closing bug.