Unreviewed, rolling out r200479.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 May 2016 23:30:21 +0000 (23:30 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 May 2016 23:30:21 +0000 (23:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=157397

A LayoutTest added with this change is crashing on Mac WK1
test runs. (Requested by ryanhaddad on #webkit).

Reverted changeset:

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

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@200494 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/fragment-activation-focuses-target-expected.txt
LayoutTests/fast/dom/fragment-activation-focuses-target.html
LayoutTests/fast/events/sequential-focus-navigation-starting-point-expected.txt [deleted file]
LayoutTests/fast/events/sequential-focus-navigation-starting-point.html [deleted file]
LayoutTests/platform/ios-simulator/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/page/EventHandler.cpp
Source/WebCore/page/FocusController.cpp
Source/WebCore/page/FrameView.cpp

index d20d1bc..04cdac1 100644 (file)
@@ -1,3 +1,18 @@
+2016-05-05  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r200479.
+        https://bugs.webkit.org/show_bug.cgi?id=157397
+
+        A LayoutTest added with this change is crashing on Mac WK1
+        test runs. (Requested by ryanhaddad on #webkit).
+
+        Reverted changeset:
+
+        "For keyboard users, activating a fragment URL should transfer
+        focus and caret to the destination"
+        https://bugs.webkit.org/show_bug.cgi?id=116046
+        http://trac.webkit.org/changeset/200479
+
 2016-05-05  Chris Dumez  <cdumez@apple.com>
 
         CORS check is sometimes incorrectly failing for media loads
index c8dab95..f1e759f 100644 (file)
@@ -14,9 +14,9 @@ PASS document.activeElement is document.getElementById('fragment1')
 Verify Tab behaves correctly after following the link.
 PASS document.activeElement is document.getElementById('fragment3')
 PASS document.activeElement is document.getElementById('fragment1')
-Activate a link that does not have a focusable fragment and verify that the currently focused element is unfocused.
+Activate a link that does not have a focusable fragment and verify focus does not move.
+PASS document.activeElement is link2
 PASS document.activeElement is link2
-PASS document.activeElement is document.body
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 77a1abe..b4c2a1e 100644 (file)
@@ -43,12 +43,12 @@ if (window.testRunner) {
   shouldBe("document.activeElement", "document.getElementById('fragment1')");
 }
 
-debug("Activate a link that does not have a focusable fragment and verify that the currently focused element is unfocused.");
+debug("Activate a link that does not have a focusable fragment and verify focus does not move.");
 var link2 = document.getElementById("link2");
 link2.focus();
 shouldBe("document.activeElement", "link2");
 link2.click();
-shouldBe("document.activeElement", "document.body");
+shouldBe("document.activeElement", "link2");
 
 var successfullyParsed = true;
 
diff --git a/LayoutTests/fast/events/sequential-focus-navigation-starting-point-expected.txt b/LayoutTests/fast/events/sequential-focus-navigation-starting-point-expected.txt
deleted file mode 100644 (file)
index 2635b4e..0000000
+++ /dev/null
@@ -1,19 +0,0 @@
-Mouse press should update sequential focus navigation starting point.
-PASS container.innerHTML = '<input id=prev><div style="height:200px;"><span>text</span></div><input id=next>'; focusSpan(); moveFocus('forward'); document.activeElement.id is 'next'
-PASS container.innerHTML = '<input id=prev><div style="height:200px;"><span>text</span></div><input id=next>'; focusSpan(); moveFocus('backward'); document.activeElement.id is 'prev'
-PASS container.innerHTML = '<span style="font-size:60px;"><input id=prev>Text Text<input id=next></span>'; focusSpan(); moveFocus('forward'); document.activeElement.id is 'next'
-PASS container.innerHTML = '<span style="font-size:60px;"><input id=prev>Text Text<input id=next></span>'; focusSpan(); moveFocus('backward'); document.activeElement.id is 'prev'
-
-Fragment navigation should update sequential focus navigation starting point.
-PASS container.innerHTML = '<a href="#fragment"></a><input id=prev><a name="fragment"></a><input id=next>'; clickLink(); moveFocus('forward'); document.activeElement.id is 'next'
-
-Focusing an element should update sequential focus navigation starting point.
-PASS container.innerHTML = '<input id=prev><input id=start><input id=next>'; focusStart(); moveFocus('forward'); document.activeElement.id is 'next'
-
-After removing a focused element from the document tree, sequential focus navigation should start at a place where the focused element was.
-PASS container.innerHTML = '<input id=prev><input id=start><input id=next>'; focusStart(); removeStart(); moveFocus('forward'); document.activeElement.id is 'next'
-PASS container.innerHTML = '<input id=prev><input id=start><input id=next>'; focusStart(); removeStart(); moveFocus('backward'); document.activeElement.id is 'prev'
-PASS container.innerHTML = '<input id=prev><div><input id=start></div><input id=next>'; focusStart(); removeDiv(); moveFocus('forward'); document.activeElement.id is 'next'
-PASS container.innerHTML = '<div><input id=start><input id=prev></div><input id=next>'; focusStart(); removeStart(); moveFocus('forward'); document.activeElement.id is 'prev'
-
-
diff --git a/LayoutTests/fast/events/sequential-focus-navigation-starting-point.html b/LayoutTests/fast/events/sequential-focus-navigation-starting-point.html
deleted file mode 100644 (file)
index e97d906..0000000
+++ /dev/null
@@ -1,69 +0,0 @@
-<!DOCTYPE html>
-<body onload="runTest();">
-<script src="../../resources/js-test-pre.js"></script>
-<script src="../forms/resources/common.js"></script>
-<div id="log"></div>
-<div id="container"></div>
-<script>
-if (!window.eventSender)
-    document.body.textContent = 'This test requires window.eventSender.';
-
-function focusSpan() { 
-    hoverOverElement(container.querySelector('span')); 
-    eventSender.mouseDown();
-}
-
-function focusDiv() { 
-    hoverOverElement(container.querySelector('div')); 
-    eventSender.mouseDown();
-}
-
-function moveFocus(direction) { 
-    eventSender.keyDown('\t', direction == 'forward' ? [] : ['shiftKey']); 
-}
-
-function clickLink() {
-    container.querySelector('a').click();
-}
-
-function focusStart() {
-    container.querySelector('#start').focus();
-}
-
-function removeStart() {
-    container.querySelector('#start').remove();
-}
-
-function removeDiv() {
-    container.querySelector('div').remove();
-}
-
-function runTest() {
-    
-    debug("Mouse press should update sequential focus navigation starting point.");
-    var container = document.querySelector('#container');
-    
-    shouldBe("container.innerHTML = '<input id=prev><div style=\"height:200px;\"><span>text</span></div><input id=next>'; focusSpan(); moveFocus('forward'); document.activeElement.id", "'next'");
-    eventSender.mouseUp();
-    shouldBe("container.innerHTML = '<input id=prev><div style=\"height:200px;\"><span>text</span></div><input id=next>'; focusSpan(); moveFocus('backward'); document.activeElement.id", "'prev'");
-    eventSender.mouseUp();
-    shouldBe("container.innerHTML = '<span style=\"font-size:60px;\"><input id=prev>Text Text<input id=next></span>'; focusSpan(); moveFocus('forward'); document.activeElement.id", "'next'");
-    eventSender.mouseUp();
-    shouldBe("container.innerHTML = '<span style=\"font-size:60px;\"><input id=prev>Text Text<input id=next></span>'; focusSpan(); moveFocus('backward'); document.activeElement.id", "'prev'");
-    eventSender.mouseUp();
-    
-    debug("\nFragment navigation should update sequential focus navigation starting point.");
-    shouldBe("container.innerHTML = '<a href=\"#fragment\"></a><input id=prev><a name=\"fragment\"></a><input id=next>'; clickLink(); moveFocus('forward'); document.activeElement.id", "'next'");
-    
-    debug("\nFocusing an element should update sequential focus navigation starting point.");
-    shouldBe("container.innerHTML = '<input id=prev><input id=start><input id=next>'; focusStart(); moveFocus('forward'); document.activeElement.id", "'next'");
-    
-    debug("\nAfter removing a focused element from the document tree, sequential focus navigation should start at a place where the focused element was.");
-    shouldBe("container.innerHTML = '<input id=prev><input id=start><input id=next>'; focusStart(); removeStart(); moveFocus('forward'); document.activeElement.id", "'next'");
-    shouldBe("container.innerHTML = '<input id=prev><input id=start><input id=next>'; focusStart(); removeStart(); moveFocus('backward'); document.activeElement.id", "'prev'");
-    shouldBe("container.innerHTML = '<input id=prev><div><input id=start></div><input id=next>'; focusStart(); removeDiv(); moveFocus('forward'); document.activeElement.id", "'next'");
-    shouldBe("container.innerHTML = '<div><input id=start><input id=prev></div><input id=next>'; focusStart(); removeStart(); moveFocus('forward'); document.activeElement.id", "'prev'");
-}
-
-</script>
-</body>
\ No newline at end of file
index f383d3d..ccf156f 100644 (file)
@@ -256,7 +256,6 @@ webkit.org/b/148695 fast/shadow-dom [ Pass ]
 
 # No tab navigation support on iOS
 fast/shadow-dom/negative-tabindex-on-shadow-host.html [ Failure ]
-webkit.org/b/116046 fast/events/sequential-focus-navigation-starting-point.html [ Skip ]
 
 webkit.org/b/150225 fast/custom-elements [ Pass ]
 
index 27f9e17..69553b5 100644 (file)
@@ -1,3 +1,18 @@
+2016-05-05  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r200479.
+        https://bugs.webkit.org/show_bug.cgi?id=157397
+
+        A LayoutTest added with this change is crashing on Mac WK1
+        test runs. (Requested by ryanhaddad on #webkit).
+
+        Reverted changeset:
+
+        "For keyboard users, activating a fragment URL should transfer
+        focus and caret to the destination"
+        https://bugs.webkit.org/show_bug.cgi?id=116046
+        http://trac.webkit.org/changeset/200479
+
 2016-05-05  Chris Dumez  <cdumez@apple.com>
 
         CORS check is sometimes incorrectly failing for media loads
index 8f6c8f7..fa7f6d0 100644 (file)
@@ -691,7 +691,6 @@ void Document::removedLastRef()
         m_activeElement = nullptr;
         m_titleElement = nullptr;
         m_documentElement = nullptr;
-        m_focusNavigationStartingNode = nullptr;
         m_userActionElements.documentDidRemoveLastRef();
 #if ENABLE(FULLSCREEN_API)
         m_fullScreenElement = nullptr;
@@ -2312,7 +2311,6 @@ void Document::destroyRenderTree()
     m_hoveredElement = nullptr;
     m_focusedElement = nullptr;
     m_activeElement = nullptr;
-    m_focusNavigationStartingNode = nullptr;
 
     if (m_documentElement)
         RenderTreeUpdater::tearDownRenderers(*m_documentElement);
@@ -3713,10 +3711,8 @@ void Document::removeFocusedNodeOfSubtree(Node* node, bool amongChildrenOnly)
     else
         nodeInSubtree = (focusedElement == node) || focusedElement->isDescendantOf(node);
     
-    if (nodeInSubtree) {
+    if (nodeInSubtree)
         setFocusedElement(nullptr);
-        setFocusNavigationStartingNode(focusedElement);
-    }
 }
 
 void Document::hoveredElementDidDetach(Element* element)
@@ -3776,7 +3772,6 @@ bool Document::setFocusedElement(Element* element, FocusDirection direction)
             oldFocusedElement->setActive(false);
 
         oldFocusedElement->setFocus(false);
-        setFocusNavigationStartingNode(nullptr);
 
         // Dispatch a change event for form control elements that have been edited.
         if (is<HTMLFormControlElement>(*oldFocusedElement)) {
@@ -3824,7 +3819,6 @@ bool Document::setFocusedElement(Element* element, FocusDirection direction)
         }
         // Set focus on the new node
         m_focusedElement = newFocusedElement;
-        setFocusNavigationStartingNode(m_focusedElement.get());
 
         // Dispatch the focus event and let the node do any other focus related activities (important for text fields)
         m_focusedElement->dispatchFocusEvent(oldFocusedElement.copyRef(), direction);
@@ -3891,59 +3885,6 @@ SetFocusedNodeDone:
     return !focusChangeBlocked;
 }
 
-static bool isNodeFrameOrDocument(Node& node)
-{
-    return is<HTMLIFrameElement>(node) || is<HTMLHtmlElement>(node) || is<HTMLDocument>(node);
-}
-
-void Document::setFocusNavigationStartingNode(Node* node)
-{
-    if (!m_frame)
-        return;
-
-    m_focusNavigationStartingNodeIsRemoved = false;
-    if (!node || isNodeFrameOrDocument(*node)) {
-        m_focusNavigationStartingNode = nullptr;
-        return;
-    }
-
-    m_focusNavigationStartingNode = node;
-}
-
-Element* Document::focusNavigationStartingNode(FocusDirection direction) const
-{
-    if (m_focusedElement) {
-        if (!m_focusNavigationStartingNode || !m_focusNavigationStartingNode->isDescendantOf(m_focusedElement.get()))
-            return m_focusedElement.get();
-    }
-
-    if (!m_focusNavigationStartingNode)
-        return nullptr;
-
-    Node* node = m_focusNavigationStartingNode.get();
-    
-    // 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);
-    }
-
-    if (is<Element>(*node))
-        return downcast<Element>(node);
-    // When going forward, the sibling needs to be an element prior to the next focusable element.
-    // Similar logic goes to the backwards case.
-    if (Element* sibling = direction == FocusDirectionForward ? ElementTraversal::previous(*node) : ElementTraversal::next(*node))
-        return sibling;
-    return node->parentOrShadowHostElement();
-}
-
 void Document::setCSSTarget(Element* n)
 {
     if (m_cssTarget)
@@ -4058,8 +3999,6 @@ void Document::nodeChildrenWillBeRemoved(ContainerNode& container)
         for (Text* textNode = TextNodeTraversal::firstChild(container); textNode; textNode = TextNodeTraversal::nextSibling(*textNode))
             m_markers->removeMarkers(textNode);
     }
-
-    updateFocusNavigationStartingNodeWithNodeRemoval(container, true);
 }
 
 void Document::nodeWillBeRemoved(Node& n)
@@ -4078,35 +4017,6 @@ void Document::nodeWillBeRemoved(Node& n)
 
     if (is<Text>(n))
         m_markers->removeMarkers(&n);
-
-    updateFocusNavigationStartingNodeWithNodeRemoval(n, false);
-}
-
-static Node* fallbackFocusNavigationStartingNodeAfterRemoval(Node& node)
-{
-    return node.previousSibling() ? node.previousSibling() : node.parentNode();
-}
-
-void Document::updateFocusNavigationStartingNodeWithNodeRemoval(Node& node, bool removeChildren)
-{
-    if (!m_focusNavigationStartingNode)
-        return;
-
-    if (m_focusNavigationStartingNode.get() == &node) {
-        if (removeChildren)
-            return;
-        m_focusNavigationStartingNode = fallbackFocusNavigationStartingNodeAfterRemoval(node);
-        m_focusNavigationStartingNodeIsRemoved = true;
-        return;
-    }
-
-    for (Node* parentNode = m_focusNavigationStartingNode->parentNode(); parentNode; parentNode = parentNode->parentNode()) {
-        if (parentNode == &node) {
-            m_focusNavigationStartingNode = removeChildren ? &node : fallbackFocusNavigationStartingNodeAfterRemoval(node);
-            m_focusNavigationStartingNodeIsRemoved = true;
-            return;
-        }
-    }
 }
 
 void Document::textInserted(Node* text, unsigned offset, unsigned length)
index ddfc901..1bcb48b 100644 (file)
@@ -729,9 +729,6 @@ public:
     UserActionElementSet& userActionElements()  { return m_userActionElements; }
     const UserActionElementSet& userActionElements() const { return m_userActionElements; }
 
-    void setFocusNavigationStartingNode(Node*);
-    Element* focusNavigationStartingNode(FocusDirection) const;
-
     void removeFocusedNodeOfSubtree(Node*, bool amongChildrenOnly = false);
     void hoveredElementDidDetach(Element*);
     void elementInActiveChainDidDetach(Element*);
@@ -771,7 +768,6 @@ public:
     void nodeChildrenWillBeRemoved(ContainerNode&);
     // nodeWillBeRemoved is only safe when removing one node at a time.
     void nodeWillBeRemoved(Node&);
-    void updateFocusNavigationStartingNodeWithNodeRemoval(Node&, bool);
     enum class AcceptChildOperation { Replace, InsertOrAdd };
     bool canAcceptChild(const Node& newChild, const Node* refChild, AcceptChildOperation) const;
 
@@ -1468,8 +1464,6 @@ private:
 
     Color m_textColor;
 
-    bool m_focusNavigationStartingNodeIsRemoved;
-    RefPtr<Node> m_focusNavigationStartingNode;
     RefPtr<Element> m_focusedElement;
     RefPtr<Element> m_hoveredElement;
     RefPtr<Element> m_activeElement;
index 5aa7fea..c773e00 100644 (file)
@@ -793,7 +793,6 @@ bool EventHandler::handleMousePressEvent(const MouseEventWithHitTestResults& eve
         focusDocumentView();
 
     m_mousePressNode = event.targetNode();
-    m_frame.document()->setFocusNavigationStartingNode(event.targetNode());
 #if ENABLE(DRAG_SUPPORT)
     m_dragStartPos = event.event().position();
 #endif
@@ -1649,7 +1648,6 @@ bool EventHandler::handleMousePressEvent(const PlatformMouseEvent& platformMouse
     }
 
     m_mousePressNode = mouseEvent.targetNode();
-    m_frame.document()->setFocusNavigationStartingNode(mouseEvent.targetNode());
 
     RefPtr<Frame> subframe = subframeForHitTestResult(mouseEvent);
     if (subframe && passMousePressEventToSubframe(mouseEvent, subframe.get())) {
index e8dd3e6..bc1043b 100644 (file)
@@ -345,7 +345,7 @@ bool FocusController::advanceFocusInDocumentOrder(FocusDirection direction, Keyb
     Frame& frame = focusedOrMainFrame();
     Document* document = frame.document();
 
-    Node* currentNode = document->focusNavigationStartingNode(direction);
+    Node* currentNode = document->focusedElement();
     // FIXME: Not quite correct when it comes to focus transitions leaving/entering the WebView itself
     bool caretBrowsing = frame.settings().caretBrowsingEnabled();
 
index 0bcc8af..edfc3cf 100644 (file)
@@ -2090,14 +2090,8 @@ bool FrameView::scrollToAnchor(const String& name)
     maintainScrollPositionAtAnchor(scrollPositionAnchor);
     
     // If the anchor accepts keyboard focus, move focus there to aid users relying on keyboard navigation.
-    if (anchorElement) {
-        if (anchorElement->isFocusable())
-            document.setFocusedElement(anchorElement);
-        else {
-            document.setFocusedElement(nullptr);
-            document.setFocusNavigationStartingNode(anchorElement);
-        }
-    }
+    if (anchorElement && anchorElement->isFocusable())
+        document.setFocusedElement(anchorElement);
     
     return true;
 }