[webkit-reviews] review denied: [Bug 60430] selectstart is not fired when selection was created by arrow keys : [Attachment 106564] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Arko Saha <nghq36 at motorola.com>'s
request for review:
Bug 60430: selectstart is not fired when selection was created by arrow keys
https://bugs.webkit.org/show_bug.cgi?id=60430

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

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=106564&action=review


> Source/WebCore/editing/FrameSelection.cpp:886
> +	   if (oldSelection.isCaret()) {
> +	       if (!(dispatchedSelectStart = dispatchSelectStart()))
> +		   break;
> +	   }

This is too late. m_selection.m_isDirectional has already been modified at this
point. r- because of this. You should probably do this in the if-statement at
the beginning of this function.

> Source/WebCore/editing/FrameSelection.cpp:1923
> +    if (isContentEditable()) {
> +	   root = highestEditableRoot(m_selection.start());
> +	   if (Node* shadowRoot = m_selection.nonBoundaryShadowTreeRootNode())
> +	       selectStartTarget = shadowRoot->shadowAncestorNode();
> +	   else
> +	       selectStartTarget = root.get();
> +    } else {
> +	   root = m_selection.nonBoundaryShadowTreeRootNode();
> +	   if (root)
> +	       selectStartTarget = root->shadowAncestorNode();
> +	   else {
> +	       root = document->documentElement();
> +	       selectStartTarget = document->body();
> +	   }
> +    }
> +    if (!root || !selectStartTarget)
> +	   return true;

Why do we need all this code?