[webkit-reviews] review requested: [Bug 28234] AXTextMarkers: AccessibilityRenderObject::boundsForVisiblePositionRange is wrong in some cases : [Attachment 34698] patch

[bugzilla-daemon at webkit.org]

chris fleizach <cfleizach at apple.com> has asked	for review:
Bug 28234: AXTextMarkers:
AccessibilityRenderObject::boundsForVisiblePositionRange is wrong in some cases
https://bugs.webkit.org/show_bug.cgi?id=28234

Attachment 34698: patch
https://bugs.webkit.org/attachment.cgi?id=34698&action=review

------- Additional Comments from chris fleizach <cfleizach at apple.com>
I'm no expert about Position(s) but here is what I found comparing two cases.

case 1) a block of text 
case 2) a block of text with a link following the text.

Goal: To retrieve the bounds of the second line of text in the block.

In the first case, [Position.cpp:983]

box = searchAheadForBetterMatch(textRenderer);

finds nothing because there's nothing else in the list of siblings. It uses its
initial m_offset as the caretPosition.. bounds are then calculated correctly.

In the second case, same line position

box = searchAheadForBetterMatch(textRenderer);

finds the InlineRenderer for the anchor tag. it uses that box's caretMinOffset,
which is zero, instead of m_offset. Now the bounds are messed up because 
we think the caret offset is back to zero

My fix, which may be incorrect (but doesn't seem to cause any regression test
failures), is that we should only reset the caretOffset to the new box's offset

if it's > than the current offset (because we're looking downstream in this
case, we shouldn't get a offset less than what we started).