Spatial navigation enhancements

Source/core/html/HTMLElement.cpp

Index: Source/core/html/HTMLElement.cpp
diff --git a/Source/core/html/HTMLElement.cpp b/Source/core/html/HTMLElement.cpp
index 485e9b0cd5da1170b084a27a69bc73ce13b3a899..acf4b816b578e60e0dc91c1e075bc4a527448807 100644
--- a/Source/core/html/HTMLElement.cpp
+++ b/Source/core/html/HTMLElement.cpp
@@ -38,6 +38,7 @@
 #include "core/dom/EventListener.h"
 #include "core/dom/EventNames.h"
 #include "core/dom/ExceptionCode.h"
+#include "core/dom/KeyboardEvent.h"
 #include "core/dom/NodeTraversal.h"
 #include "core/dom/Text.h"
 #include "core/editing/markup.h"
@@ -48,6 +49,7 @@
 #include "core/html/parser/HTMLParserIdioms.h"
 #include "core/loader/FrameLoader.h"
 #include "core/page/Frame.h"
+#include "core/page/Settings.h"
 #include "core/rendering/RenderWordBreak.h"
 #include <wtf/StdLibExtras.h>
 #include <wtf/text/CString.h>
@@ -598,9 +600,19 @@ bool HTMLElement::hasCustomFocusLogic() const
     return false;
 }
 
+static inline bool hasListenerForClickOrKeyEvent(const HTMLElement* element)
+{
+    EventTarget* target = const_cast<HTMLElement*>(element);

[esprehn, 2013/06/14 08:01:24]

I don't understand why you need this cast. Just call hasEventListeners directly?
It'd be better if this was just a private method on HTMLElement and you put the
spatialNavigationEnabled() check in there too.

supportsFocus() would do return ... || supportsSpatialNavigationFocus();

+    return target->hasEventListeners(eventNames().clickEvent)
+        || target->hasEventListeners(eventNames().keydownEvent)
+        || target->hasEventListeners(eventNames().keypressEvent)
+        || target->hasEventListeners(eventNames().keyupEvent);
+}
+
 bool HTMLElement::supportsFocus() const
 {
-    return Element::supportsFocus() || (rendererIsEditable() && parentNode() && !parentNode()->rendererIsEditable());
+    return Element::supportsFocus() || (rendererIsEditable() && parentNode() && !parentNode()->rendererIsEditable())
+        || document()->settings()->spatialNavigationEnabled() && hasListenerForClickOrKeyEvent(this);
 }
 
 String HTMLElement::contentEditable() const
@@ -1024,6 +1036,29 @@ void HTMLElement::addHTMLColorToStyle(MutableStylePropertySet* style, CSSPropert
     style->setProperty(propertyID, cssValuePool().createColorValue(parsedColor.rgb()));
 }
 
+void HTMLElement::defaultEventHandler(Event* evt)

[esprehn, 2013/06/14 08:01:24]

event. Please don't abbreviate.

+{
+    if (evt->isKeyboardEvent() && evt->type() == eventNames().keypressEvent) {

[esprehn, 2013/06/14 08:01:24]

I would put the type() check first so you can avoid the virtual call to
isKeyboardEvent() in the common case.

+        handleKeypressEvent(static_cast<KeyboardEvent*>(evt));

[esprehn, 2013/06/14 08:01:24]

Please add a toKeyboardEvent() function, they're in lots of headers. It should
ASSERT_WITH_SECURITY_IMPLICATION that event->isKeyboardEvent().

+        if (evt->defaultHandled())
+            return;
+    }
+
+    StyledElement::defaultEventHandler(evt);
+}
+
+void HTMLElement::handleKeypressEvent(KeyboardEvent* event)
+{
+    int charCode = event->charCode();
+    if (charCode == '\r' || charCode == ' ') {
+        if (document()->settings()->spatialNavigationEnabled() && supportsFocus()) {

[esprehn, 2013/06/14 08:01:24]

The spatial navigation check should be outside this if() and you should early
return from the function if either that is false or supportsFocus() is false.

+            dispatchSimulatedClick(event);
+            event->setDefaultHandled();
+            return;

[esprehn, 2013/06/14 08:01:24]

Return is not needed.

+        }
+    }
+}
+
 } // namespace WebCore
 
 #ifndef NDEBUG