[Webkit-unassigned] [Bug 116046] For keyboard users, activating a fragment URL should transfer focus and caret to the destination

[bugzilla-daemon at webkit.org]

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

--- Comment #52 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 280787
  --> https://bugs.webkit.org/attachment.cgi?id=280787
patch

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

> Source/WebCore/dom/Document.cpp:3707
> +static bool nodeInSubtree(Node* node, Node* container, bool amongChildrenOnly)

We should give this function a more descriptive name such as isNodeInSubtree.

> Source/WebCore/dom/Document.cpp:3729
>          setFocusedElement(nullptr, FocusDirectionNone, FocusRemovalEventsMode::DoNotDispatch);
> +        setFocusNavigationStartingNode(focusedElement);

We should a comment clarifying here why we need to set the focus navigation starting node to the focused element here
as well as why we need to call this function first before calling removeFocusNavigationNodeOfSubtree

> Source/WebCore/dom/Document.cpp:3912
> +    // Setting focus navigation starting node to the following nodes means that we should start
> +    // the search from the beginning of the document/frame.
> +    return is<HTMLIFrameElement>(node) || is<HTMLHtmlElement>(node) || is<HTMLDocument>(node);

This makes no sense to me. If a focus happens to be at an iframe,
why do we want to start at the beginning of a document which contains that iframe?

> Source/WebCore/dom/Document.cpp:3952
> +    // When the node was removed from the document tree. This case is not specified in the spec:
> +    // https://html.spec.whatwg.org/multipage/interaction.html#sequential-focus-navigation-starting-point
> +    // Current behaivor is to move the sequential navigation node to / after (based on the focus direction)
> +    // the previous sibling of the removed node.
> +    if (m_focusNavigationStartingNodeIsRemoved) {
> +        Node* nextNode = NodeTraversal::next(*node);
> +        if (direction == FocusDirectionForward)
> +            return ElementTraversal::previous(*nextNode);
> +        if (is<Element>(*nextNode))
> +            return downcast<Element>(nextNode);
> +        return ElementTraversal::next(*nextNode);
> +    }

Why don't we do this in removeFocusNavigationNodeOfSubtree instead?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160608/74d484a6/attachment.html>