Focus unfocusable node upon reference fragment navigation

Source/core/frame/FrameView.cpp

Index: Source/core/frame/FrameView.cpp
diff --git a/Source/core/frame/FrameView.cpp b/Source/core/frame/FrameView.cpp
index 45e333d40163e98382cb41ec151153ddfe6c8ff5..43bf342079bf7cee34bf6509325c78cc4af6bb19 100644
--- a/Source/core/frame/FrameView.cpp
+++ b/Source/core/frame/FrameView.cpp
@@ -1362,7 +1362,8 @@ bool FrameView::scrollToAnchor(const String& name)
     maintainScrollPositionAtAnchor(anchorNode ? static_cast<Node*>(anchorNode) : m_frame->document());
 
     // If the anchor accepts keyboard focus, move focus there to aid users relying on keyboard navigation.
-    if (anchorNode && anchorNode->isFocusable())
+    // If it does not accept keyboard focus, then still remove the focus for consistency with other browsers (http://crbug.com/417636)

[dmazzoni, 2014/10/08 18:19:06]

This comment doesn't make sense - you say "remove the focus", but your change
seems to *set* the focus to the anchor node.

We don't normally link to bugs in comments unless it's an open bug or temporary
hack.

[robwu, 2014/10/08 20:10:23]

setFocusedElement first unfocuses the old element. Then, iff the node argument
is focusable, then the node will be used as the new focused element. Otherwise
the node will not be stored, see
https://chromium.googlesource.com/chromium/blink/+/31dab16c269e2de356a920e5d2...
I'll remove the link at the next patchset.

[dmazzoni, 2014/10/08 20:26:27]

Oh, I get it now. I'd probably write the comment something like this:

// If anchorNode is not focusable, setFocusedElement() will still clear focus,
which matches the behavior of other browsers.

[robwu, 2014/10/09 06:12:57]

Done.

+    if (anchorNode)
         m_frame->document()->setFocusedElement(anchorNode);
 
     return true;