[Webkit-unassigned] [Bug 91912] Adding "all" to -webkit-user-select

[bugzilla-daemon at webkit.org]

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





--- Comment #41 from Ryosuke Niwa <rniwa at webkit.org>  2012-08-17 14:34:22 PST ---
(From update of attachment 159176)
View in context: https://bugs.webkit.org/attachment.cgi?id=159176&action=review

> Source/WebCore/page/EventHandler.cpp:827
> +        // movements is inside user select all area

I don't think this comment adds much value.

> Source/WebCore/page/EventHandler.cpp:828
> +        newSelection.setBase(Position(rootUserSelectAllForMousePressNode, Position::PositionIsBeforeAnchor));

You should just use positionBeforeNode.

> Source/WebCore/page/EventHandler.cpp:829
> +        newSelection.setExtent(Position(rootUserSelectAllForMousePressNode, Position::PositionIsAfterAnchor));

positionAfterNode.

> Source/WebCore/page/EventHandler.cpp:832
> +    } else {
> +        // reset base for user select all when necessary
> +        if (rootUserSelectAllForMousePressNode && comparePositions(target->renderer()->positionForPoint(hitTestResult.localPoint()), m_mousePressNode->renderer()->positionForPoint(m_dragStartPos)) < 0)

I'd prefer using else if here.

> Source/WebCore/page/EventHandler.cpp:834
> +        newSelection.setExtent(targetPosition);

Don't we need to set this to the beginning of rootUserSelectAllForMousePressNode when the previous if condition is true?

> LayoutTests/editing/selection/user-select-all-selection.html:20
> +                log("extending forward: selection should be the entire user-select-all area, which is (body, 1, body, 2).");

This is redundant since it just repeats the assert. We need to explain why it should be at (body, 1, body, 2).

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