Focus unfocusable node upon reference fragment navigation

Source/core/frame/FrameView.cpp

 /*
  * Copyright (C) 1998, 1999 Torben Weis <weis@kde.org>
  *                     1999 Lars Knoll <knoll@kde.org>
  *                     1999 Antti Koivisto <koivisto@kde.org>
  *                     2000 Dirk Mueller <mueller@kde.org>
  * Copyright (C) 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
  *           (C) 2006 Graham Dennis (graham.dennis@gmail.com)
  *           (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
@@ skipping 1344 matching lines @@
         }
     }
 
     // Implement the rule that "" and "top" both mean top of page as in other browsers.
     if (!anchorNode && !(name.isEmpty() || equalIgnoringCase(name, "top")))
         return false;
 
     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;
 }
 
 void FrameView::maintainScrollPositionAtAnchor(Node* anchorNode)
 {
     m_maintainScrollPositionAnchor = anchorNode;
     if (!m_maintainScrollPositionAnchor)
         return;
@@ skipping 1655 matching lines @@
     IntSize visibleSize = expandedIntSize(visibleContentSizeF);
 
     IntPoint maximumOffset(
         contentsWidth() - visibleSize.width() - scrollOrigin().x(),
         contentsHeight() - visibleSize.height() - scrollOrigin().y());
     maximumOffset.clampNegativeToZero();
     return maximumOffset;
 }
 
 } // namespace blink