Changeset 201832 in webkit


Ignore:
Timestamp:
Jun 8, 2016, 2:15:09 PM (10 years ago)
Author:
n_wang@apple.com
Message:

For keyboard users, activating a fragment URL should transfer focus and caret to the destination
​https://bugs.webkit.org/show_bug.cgi?id=116046

Reviewed by Ryosuke Niwa.

Source/WebCore:

Added a sequential focus navigation starting node to document. When TAB or SHIFT-TAB is pressed
and there is no focused element, we start searching for next focus candidates at the sequential
focus navigation node.
Spec: ​https://html.spec.whatwg.org/multipage/interaction.html#sequential-focus-navigation-starting-point

Test: fast/events/sequential-focus-navigation-starting-point.html

  • dom/Document.cpp:

(WebCore::Document::removedLastRef):
(WebCore::Document::destroyRenderTree):
(WebCore::Document::styleResolverChanged):
(WebCore::isNodeInSubtree):
(WebCore::Document::removeFocusedNodeOfSubtree):
(WebCore::Document::hoveredElementDidDetach):
(WebCore::Document::setFocusedElement):
(WebCore::shouldResetFocusNavigationStartingNode):
(WebCore::Document::setFocusNavigationStartingNode):
(WebCore::Document::focusNavigationStartingNode):
(WebCore::Document::setCSSTarget):
(WebCore::Document::nodeChildrenWillBeRemoved):
(WebCore::Document::nodeWillBeRemoved):
(WebCore::fallbackFocusNavigationStartingNodeAfterRemoval):
(WebCore::Document::removeFocusNavigationNodeOfSubtree):
(WebCore::Document::textInserted):

  • dom/Document.h:

(WebCore::Document::userActionElements):

  • page/EventHandler.cpp:

(WebCore::EventHandler::handleMousePressEvent):

  • page/FocusController.cpp:

(WebCore::FocusController::advanceFocusInDocumentOrder):

  • page/FrameView.cpp:

(WebCore::FrameView::scrollToAnchor):

LayoutTests:

Added a layout test to check that mouse pressing, fragment navigation, focusing an element and removing
the focused element will give us the expected focus navigation starting point.

Also updated the fragment activation test because now that navigating to an unfocusable fragment will
unfocus the current focused element.

  • fast/dom/fragment-activation-focuses-target-expected.txt:
  • fast/dom/fragment-activation-focuses-target.html:
  • fast/events/sequential-focus-navigation-starting-point-expected.txt: Added.
  • fast/events/sequential-focus-navigation-starting-point.html: Added.
  • platform/ios-simulator/TestExpectations:
Location:
trunk
Files:
2 added
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r201823 r201832  
     12016-06-08  Nan Wang  <n_wang@apple.com>
     2
     3        For keyboard users, activating a fragment URL should transfer focus and caret to the destination
     4        https://bugs.webkit.org/show_bug.cgi?id=116046
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Added a layout test to check that mouse pressing, fragment navigation, focusing an element and removing
     9        the focused element will give us the expected focus navigation starting point.
     10
     11        Also updated the fragment activation test because now that navigating to an unfocusable fragment will
     12        unfocus the current focused element.
     13
     14        * fast/dom/fragment-activation-focuses-target-expected.txt:
     15        * fast/dom/fragment-activation-focuses-target.html:
     16        * fast/events/sequential-focus-navigation-starting-point-expected.txt: Added.
     17        * fast/events/sequential-focus-navigation-starting-point.html: Added.
     18        * platform/ios-simulator/TestExpectations:
     19
    1202016-06-07  Ryosuke Niwa  <rniwa@webkit.org>
    221

  • trunk/LayoutTests/fast/dom/fragment-activation-focuses-target-expected.txt

    r200494 r201832  
    1515PASS document.activeElement is document.getElementById('fragment3')
    1616PASS document.activeElement is document.getElementById('fragment1')
    17 Activate a link that does not have a focusable fragment and verify focus does not move.
     17Activate a link that does not have a focusable fragment and verify that the currently focused element is unfocused.
    1818PASS document.activeElement is link2
    19 PASS document.activeElement is link2
     19PASS document.activeElement is document.body
    2020PASS successfullyParsed is true
    2121

  • trunk/LayoutTests/fast/dom/fragment-activation-focuses-target.html

    r200494 r201832  
    4444}
    4545
    46 debug("Activate a link that does not have a focusable fragment and verify focus does not move.");
     46debug("Activate a link that does not have a focusable fragment and verify that the currently focused element is unfocused.");
    4747var link2 = document.getElementById("link2");
    4848link2.focus();
    4949shouldBe("document.activeElement", "link2");
    5050link2.click();
    51 shouldBe("document.activeElement", "link2");
     51shouldBe("document.activeElement", "document.body");
    5252
    5353var successfullyParsed = true;

  • trunk/LayoutTests/platform/ios-simulator/TestExpectations

    r201736 r201832  
    264264fast/shadow-dom/focus-on-iframe.html [ Failure ]
    265265fast/shadow-dom/negative-tabindex-on-shadow-host.html [ Failure ]
     266webkit.org/b/116046 fast/events/sequential-focus-navigation-starting-point.html [ Skip ]
    266267
    267268webkit.org/b/150225 fast/custom-elements [ Pass ]

  • trunk/Source/WebCore/ChangeLog

    r201831 r201832  
     12016-06-08  Nan Wang  <n_wang@apple.com>
     2
     3        For keyboard users, activating a fragment URL should transfer focus and caret to the destination
     4        https://bugs.webkit.org/show_bug.cgi?id=116046
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Added a sequential focus navigation starting node to document. When TAB or SHIFT-TAB is pressed
     9        and there is no focused element, we start searching for next focus candidates at the sequential
     10        focus navigation node.
     11        Spec: https://html.spec.whatwg.org/multipage/interaction.html#sequential-focus-navigation-starting-point
     12
     13        Test: fast/events/sequential-focus-navigation-starting-point.html
     14
     15        * dom/Document.cpp:
     16        (WebCore::Document::removedLastRef):
     17        (WebCore::Document::destroyRenderTree):
     18        (WebCore::Document::styleResolverChanged):
     19        (WebCore::isNodeInSubtree):
     20        (WebCore::Document::removeFocusedNodeOfSubtree):
     21        (WebCore::Document::hoveredElementDidDetach):
     22        (WebCore::Document::setFocusedElement):
     23        (WebCore::shouldResetFocusNavigationStartingNode):
     24        (WebCore::Document::setFocusNavigationStartingNode):
     25        (WebCore::Document::focusNavigationStartingNode):
     26        (WebCore::Document::setCSSTarget):
     27        (WebCore::Document::nodeChildrenWillBeRemoved):
     28        (WebCore::Document::nodeWillBeRemoved):
     29        (WebCore::fallbackFocusNavigationStartingNodeAfterRemoval):
     30        (WebCore::Document::removeFocusNavigationNodeOfSubtree):
     31        (WebCore::Document::textInserted):
     32        * dom/Document.h:
     33        (WebCore::Document::userActionElements):
     34        * page/EventHandler.cpp:
     35        (WebCore::EventHandler::handleMousePressEvent):
     36        * page/FocusController.cpp:
     37        (WebCore::FocusController::advanceFocusInDocumentOrder):
     38        * page/FrameView.cpp:
     39        (WebCore::FrameView::scrollToAnchor):
     40
    1412016-06-08  Eric Carlson  <eric.carlson@apple.com>
    242

  • trunk/Source/WebCore/dom/Document.cpp

    r201799 r201832  
    681681        m_titleElement = nullptr;
    682682        m_documentElement = nullptr;
     683        m_focusNavigationStartingNode = nullptr;
    683684        m_userActionElements.documentDidRemoveLastRef();
    684685#if ENABLE(FULLSCREEN_API)
    … …  
    23092310    m_focusedElement = nullptr;
    23102311    m_activeElement = nullptr;
     2312    m_focusNavigationStartingNode = nullptr;
    23112313
    23122314    if (m_documentElement)
    … …  
    36943696}
    36953697
     3698static bool isNodeInSubtree(Node* node, Node* container, bool amongChildrenOnly)
     3699{
     3700    bool nodeInSubtree = false;
     3701    if (amongChildrenOnly)
     3702        nodeInSubtree = node->isDescendantOf(container);
     3703    else
     3704        nodeInSubtree = (node == container) || node->isDescendantOf(container);
     3705   
     3706    return nodeInSubtree;
     3707}
     3708
    36963709void Document::removeFocusedNodeOfSubtree(Node* node, bool amongChildrenOnly)
    36973710{
    … …  
    37023715    if (!focusedElement)
    37033716        return;
    3704 
    3705     bool nodeInSubtree = false;
    3706     if (amongChildrenOnly)
    3707         nodeInSubtree = focusedElement->isDescendantOf(node);
    3708     else
    3709         nodeInSubtree = (focusedElement == node) || focusedElement->isDescendantOf(node);
    37103717   
    3711     if (nodeInSubtree)
     3718    if (isNodeInSubtree(focusedElement, node, amongChildrenOnly)) {
    37123719        setFocusedElement(nullptr, FocusDirectionNone, FocusRemovalEventsMode::DoNotDispatch);
     3720        // Set the focus navigation starting node to the previous focused element so that
     3721        // we can fallback to the siblings or parent node for the next search.
     3722        // Also we need to call removeFocusNavigationNodeOfSubtree after this function because
     3723        // setFocusedElement(nullptr) will reset m_focusNavigationStartingNode.
     3724        setFocusNavigationStartingNode(focusedElement);
     3725    }
    37133726}
    37143727
    … …  
    37703783
    37713784        oldFocusedElement->setFocus(false);
     3785        setFocusNavigationStartingNode(nullptr);
    37723786
    37733787        if (eventsMode == FocusRemovalEventsMode::Dispatch) {
    … …  
    38203834        // Set focus on the new node
    38213835        m_focusedElement = newFocusedElement;
     3836        setFocusNavigationStartingNode(m_focusedElement.get());
    38223837
    38233838        // Dispatch the focus event and let the node do any other focus related activities (important for text fields)
    … …  
    38863901}
    38873902
     3903static bool shouldResetFocusNavigationStartingNode(Node& node)
     3904{
     3905    // Setting focus navigation starting node to the following nodes means that we should start
     3906    // the search from the beginning of the document.
     3907    return is<HTMLHtmlElement>(node) || is<HTMLDocument>(node);
     3908}
     3909
     3910void Document::setFocusNavigationStartingNode(Node* node)
     3911{
     3912    if (!m_frame)
     3913        return;
     3914
     3915    m_focusNavigationStartingNodeIsRemoved = false;
     3916    if (!node || shouldResetFocusNavigationStartingNode(*node)) {
     3917        m_focusNavigationStartingNode = nullptr;
     3918        return;
     3919    }
     3920
     3921    m_focusNavigationStartingNode = node;
     3922}
     3923
     3924Element* Document::focusNavigationStartingNode(FocusDirection direction) const
     3925{
     3926    if (m_focusedElement) {
     3927        if (!m_focusNavigationStartingNode || !m_focusNavigationStartingNode->isDescendantOf(m_focusedElement.get()))
     3928            return m_focusedElement.get();
     3929    }
     3930
     3931    if (!m_focusNavigationStartingNode)
     3932        return nullptr;
     3933
     3934    Node* node = m_focusNavigationStartingNode.get();
     3935   
     3936    // When the node was removed from the document tree. This case is not specified in the spec:
     3937    // https://html.spec.whatwg.org/multipage/interaction.html#sequential-focus-navigation-starting-point
     3938    // Current behaivor is to move the sequential navigation node to / after (based on the focus direction)
     3939    // the previous sibling of the removed node.
     3940    if (m_focusNavigationStartingNodeIsRemoved) {
     3941        Node* nextNode = NodeTraversal::next(*node);
     3942        if (direction == FocusDirectionForward)
     3943            return ElementTraversal::previous(*nextNode);
     3944        if (is<Element>(*nextNode))
     3945            return downcast<Element>(nextNode);
     3946        return ElementTraversal::next(*nextNode);
     3947    }
     3948
     3949    if (is<Element>(*node))
     3950        return downcast<Element>(node);
     3951    if (Element* elementBeforeNextFocusableElement = direction == FocusDirectionForward ? ElementTraversal::previous(*node) : ElementTraversal::next(*node))
     3952        return elementBeforeNextFocusableElement;
     3953    return node->parentOrShadowHostElement();
     3954}
     3955
    38883956void Document::setCSSTarget(Element* n)
    38893957{
    … …  
    39834051
    39844052    removeFocusedNodeOfSubtree(&container, true /* amongChildrenOnly */);
     4053    removeFocusNavigationNodeOfSubtree(container, true /* amongChildrenOnly */);
    39854054
    39864055#if ENABLE(FULLSCREEN_API)
    … …  
    40154084
    40164085    removeFocusedNodeOfSubtree(&n);
     4086    removeFocusNavigationNodeOfSubtree(n);
    40174087
    40184088#if ENABLE(FULLSCREEN_API)
    … …  
    40344104    if (is<Text>(n))
    40354105        m_markers->removeMarkers(&n);
     4106}
     4107
     4108static Node* fallbackFocusNavigationStartingNodeAfterRemoval(Node& node)
     4109{
     4110    return node.previousSibling() ? node.previousSibling() : node.parentNode();
     4111}
     4112
     4113void Document::removeFocusNavigationNodeOfSubtree(Node& node, bool amongChildrenOnly)
     4114{
     4115    if (!m_focusNavigationStartingNode)
     4116        return;
     4117
     4118    if (isNodeInSubtree(m_focusNavigationStartingNode.get(), &node, amongChildrenOnly)) {
     4119        m_focusNavigationStartingNode = amongChildrenOnly ? &node : fallbackFocusNavigationStartingNodeAfterRemoval(node);
     4120        m_focusNavigationStartingNodeIsRemoved = true;
     4121    }
    40364122}
    40374123

  • trunk/Source/WebCore/dom/Document.h

    r201799 r201832  
    731731    const UserActionElementSet& userActionElements() const { return m_userActionElements; }
    732732
     733    void setFocusNavigationStartingNode(Node*);
     734    Element* focusNavigationStartingNode(FocusDirection) const;
     735
    733736    void removeFocusedNodeOfSubtree(Node*, bool amongChildrenOnly = false);
    734737    void hoveredElementDidDetach(Element*);
    … …  
    770773    // nodeWillBeRemoved is only safe when removing one node at a time.
    771774    void nodeWillBeRemoved(Node&);
     775    void removeFocusNavigationNodeOfSubtree(Node&, bool amongChildrenOnly = false);
    772776    enum class AcceptChildOperation { Replace, InsertOrAdd };
    773777    bool canAcceptChild(const Node& newChild, const Node* refChild, AcceptChildOperation) const;
    … …  
    14661470    Color m_textColor;
    14671471
     1472    bool m_focusNavigationStartingNodeIsRemoved;
     1473    RefPtr<Node> m_focusNavigationStartingNode;
    14681474    RefPtr<Element> m_focusedElement;
    14691475    RefPtr<Element> m_hoveredElement;

  • trunk/Source/WebCore/page/EventHandler.cpp

    r201588 r201832  
    796796
    797797    m_mousePressNode = event.targetNode();
     798    m_frame.document()->setFocusNavigationStartingNode(event.targetNode());
    798799#if ENABLE(DRAG_SUPPORT)
    799800    m_dragStartPos = event.event().position();
    … …  
    16551656
    16561657    m_mousePressNode = mouseEvent.targetNode();
     1658    m_frame.document()->setFocusNavigationStartingNode(mouseEvent.targetNode());
    16571659
    16581660    RefPtr<Frame> subframe = subframeForHitTestResult(mouseEvent);

  • trunk/Source/WebCore/page/FocusController.cpp

    r200964 r201832  
    455455    Document* document = frame.document();
    456456
    457     Node* currentNode = document->focusedElement();
     457    Node* currentNode = document->focusNavigationStartingNode(direction);
    458458    // FIXME: Not quite correct when it comes to focus transitions leaving/entering the WebView itself
    459459    bool caretBrowsing = frame.settings().caretBrowsingEnabled();

  • trunk/Source/WebCore/page/FrameView.cpp

    r201673 r201832  
    21012101   
    21022102    // If the anchor accepts keyboard focus, move focus there to aid users relying on keyboard navigation.
    2103     if (anchorElement && anchorElement->isFocusable())
    2104         document.setFocusedElement(anchorElement);
     2103    if (anchorElement) {
     2104        if (anchorElement->isFocusable())
     2105            document.setFocusedElement(anchorElement);
     2106        else {
     2107            document.setFocusedElement(nullptr);
     2108            document.setFocusNavigationStartingNode(anchorElement);
     2109        }
     2110    }
    21052111   
    21062112    return true;
Note: See TracChangeset for help on using the changeset viewer.