[go: up one dir, main page]

[SpatNav]: Make {cursor: pointer} trees focus candidates

Background:
Some web apps use one "catch all"-click handler, often put
on <body> or a container <div>, to react on clicks within
certain areas of the page. These web pages expect clicks
(or touches) and are not designed with key[board] users in
mind (they lack tabindex).

These pages' clickables, for example <div>s that look like
buttons, are often styled with {cursor: pointer}.

Problem:
These web pages are inaccessible for keyboard-only users.

Solution:
Enable SpatNav on these web apps by adding {cursor: pointer}
sub trees' tip to the list of navigation candidates.

Spec discussion:
https://github.com/w3c/csswg-drafts/issues/3395

Note:
snav-imagemap-area-not-focusable.html found a bug in
Chrome's UA CSS. We used to style *all* <area>s as
clickables, with {cursor: pointer}, even those without a
href-attribute. I fixed this in crrev.com/c/1653009.

(cherry picked from commit 308a30020307ae3925507536c48e5450c55b21f2)

(cherry picked from commit b2f29c71262890c28fbd7fdee139ba9af5c4a9ee)

Bug: 961927
Change-Id: I7baa2ba6f5f36ebc23b072d677edc66acfb2a70b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1632231
Commit-Queue: Hugo Holgersson <hholgersson@fb.com>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/master@{#669193}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1662868
Cr-Original-Commit-Position: refs/branch-heads/3809@{#375}
Cr-Original-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1662837
Cr-Commit-Position: refs/branch-heads/3770@{#1032}
Cr-Branched-From: a9eee1c7c727ef42a15d86e9fa7b77ff0e63840a-refs/heads/master@{#652427}
diff --git a/third_party/blink/renderer/core/dom/element.cc b/third_party/blink/renderer/core/dom/element.cc
index d86329b..e5df6bb 100644
--- a/third_party/blink/renderer/core/dom/element.cc
+++ b/third_party/blink/renderer/core/dom/element.cc
@@ -3491,11 +3491,26 @@
   // events).
   if (!IsSpatialNavigationEnabled(GetDocument().GetFrame()))
     return false;
+
+  if (!GetLayoutObject())
+    return false;
+
   if (HasEventListeners(event_type_names::kClick) ||
       HasEventListeners(event_type_names::kKeydown) ||
       HasEventListeners(event_type_names::kKeypress) ||
       HasEventListeners(event_type_names::kKeyup))
     return true;
+
+  // Some web apps use click-handlers to react on clicks within rects that are
+  // styled with {cursor: pointer}. Such rects *look* clickable so they probably
+  // are. Here we make Hand-trees' tip, the first (biggest) node with {cursor:
+  // pointer}, navigable because users shouldn't need to navigate through every
+  // sub element that inherit this CSS.
+  if (GetComputedStyle()->Cursor() == ECursor::kPointer &&
+      ParentComputedStyle()->Cursor() != ECursor::kPointer) {
+    return true;
+  }
+
   if (!IsSVGElement())
     return false;
   return (HasEventListeners(event_type_names::kFocus) ||
diff --git a/third_party/blink/web_tests/fast/spatial-navigation/snav-cursor_pointer-clickable.html b/third_party/blink/web_tests/fast/spatial-navigation/snav-cursor_pointer-clickable.html
new file mode 100644
index 0000000..91b7905f
--- /dev/null
+++ b/third_party/blink/web_tests/fast/spatial-navigation/snav-cursor_pointer-clickable.html
@@ -0,0 +1,29 @@
+<!doctype html>
+<style>
+  #popup {width: 300px; height: 150px; background: LightBlue;}
+  .button {font-family: sans-serif; font-weight: bold; width: 80px; height: 40px; background: SeaGreen; color: white; cursor: pointer;}
+  .balancer {display: flex; justify-content: space-evenly; align-items: center;}
+</style>
+
+<div class="balancer" id="popup">
+  <div class="button balancer" id="ok"><div id="dontgohere1">OK</div></div>
+  <div class="button balancer" id="cancel"><div id="dontgohere2">Cancel</div></div>
+</div>
+
+<div id="log"><div>
+
+<p>A typical popup dialog that listens for clicks on its two custom buttons.</p>
+
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<script src="resources/snav-testharness.js"></script>
+<script>
+  var resultMap = [
+    ["Down", "ok"],
+    ["Right", "cancel"],
+    ["Left", "ok"],
+  ];
+  snav.assertFocusMoves(resultMap);
+</script>
+
+<p><em>Manual test instruction: Ensure that all buttons can be focused.</em></p>