[Webkit-unassigned] [Bug 105189] Add function to move caret selection towards a point

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=105189


Ryosuke Niwa <rniwa at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #179763|review?                     |review-
               Flag|                            |




--- Comment #8 from Ryosuke Niwa <rniwa at webkit.org>  2012-12-18 10:13:36 PST ---
(From update of attachment 179763)
View in context: https://bugs.webkit.org/attachment.cgi?id=179763&action=review

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1398
> +    VisiblePosition movePosition = frame()->selection()->selection().visibleStart();

This should be visibleBase();

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1401
> +    IntRect editableRect = editable->renderer()->enclosingLayer()->absoluteBoundingBox();

This is not right. You need to obtain the bounding box of the roo editable element.

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1405
> +    bool movingUp = point.y < windowMovePosition.y();

This is wrong when we're in a vertical writing mode. r- because of this.

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1407
> +        VisiblePosition newMovePosition = movingUp ? previousLinePosition(movePosition, 0) : nextLinePosition(movePosition, 0);

Ditto.

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1411
> +        if (movingUp ? point.y > windowNewMovePosition.y() : point.y < windowNewMovePosition.y())

Ditto.

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1414
> +        if (movingUp ? editableRect.y() > windowNewMovePosition.y(): editableRect.y() + editableRect.height() < windowNewMovePosition.y())

Ditto.

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1419
> +    bool movingLeft = point.x < windowMovePosition.x();

Ditto.

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1421
> +        VisiblePosition newMovePosition = movingLeft ? movePosition.left(true) : movePosition.right(true);

Ditto.

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1425
> +        if (movingLeft ? point.x > windowNewMovePosition.x() : point.x < windowNewMovePosition.x())

Ditto.

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1428
> +        if (movingLeft ? editableRect.x() > windowNewMovePosition.x(): editableRect.x() + editableRect.width() < windowNewMovePosition.x())

Ditto.

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1432
> +    frame()->selection()->moveTo(movePosition, UserTriggered);

You need to call FrameSelection::shouldChangeSelection before calling moveTo.
I'd suggest adding this entire function to FrameSelection instead.

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1434
> +    if (viewImpl()->client())
> +        viewImpl()->client()->didChangeSelection(true);

Why are we directly calling back to didChangeSelection? moveTo calls FrameSelection::setSelection, which in turn calls Editor::respondToChangedSelection and then EditorClient::respondToChangedSelection.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.