[Contextual Search] Check ARIA Roles before triggering.

content/renderer/render_widget.cc

Index: content/renderer/render_widget.cc
diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc
index c2eefbd766a8c00c252c1c06c249675af13d37fa..a56f8085fb2546ebf6debcc4d9b9fcbdfbe22313 100644
--- a/content/renderer/render_widget.cc
+++ b/content/renderer/render_widget.cc
@@ -68,6 +68,7 @@
 #include "third_party/WebKit/public/web/WebRange.h"
 #include "third_party/WebKit/public/web/WebRuntimeFeatures.h"
 #include "third_party/skia/include/core/SkShader.h"
+#include "ui/accessibility/ax_enums.h"
 #include "ui/base/ui_base_switches.h"
 #include "ui/gfx/frame_time.h"
 #include "ui/gfx/geometry/point_conversions.h"
@@ -2137,13 +2138,77 @@ void RenderWidget::showUnhandledTapUIIfNeeded(
     const WebNode& tapped_node,
     bool page_changed) {
   DCHECK(handling_input_event_);
+  WebCursor::CursorInfo cursor_info;
+  current_cursor_.GetCursorInfo(&cursor_info);

[dmazzoni, 2015/01/23 17:39:56]

I haven't used current_cursor_ before - are we sure that this is always the
computed cursor of |tapped_node|?

[Donn Denman, 2015/01/29 22:42:39]

I couldn't figure out the cursor stuff, so decided to drop it from this CL.

+  bool has_pointer_cursor = cursor_info.type == WebCursorInfo::TypeHand;
+  bool has_wai_aria_role = hasAriaRole(tapped_node);
   bool should_trigger = !page_changed && tapped_node.isTextNode() &&
-                        !tapped_node.isContentEditable();
+                        !tapped_node.isContentEditable() &&
+                        !has_wai_aria_role && !tapped_node.isFocusable() &&

[dmazzoni, 2015/01/23 17:39:56]

I think you need to search up the ancestor chain to see if an ancestor is
focusable, because the tap could be on an element inside a focusable element.

Also, note that text nodes are not elements and so they can't be focusable.

[Donn Denman, 2015/01/29 22:42:39]

Done -- Now checking all ancestors for focusability.

+                        !has_pointer_cursor;
   if (should_trigger) {
     Send(new ViewHostMsg_ShowUnhandledTapUIIfNeeded(routing_id_,
         tapped_position.x, tapped_position.y));
   }
 }
+
+bool RenderWidget::hasAriaRole(const WebNode& node) {

[dmazzoni, 2015/01/23 17:39:56]

This should probably be named hasWidgetAriaRole or something like that.

[Donn Denman, 2015/01/29 22:42:39]

Moved this logic into Blink.

+  // From http://www.w3.org/TR/wai-aria/roles#widget_roles
+  const ui::AXRole widget_and_composite_roles[] = {
+      ui::AX_ROLE_ALERT,

[dmazzoni, 2015/01/23 17:39:56]

ui::AXRole is not a list of ARIA roles, it's a list of all accessible roles,
which is a superset of the ARIA roles. As an example, the reason you didn't find
ui::AX_ROLE_GRID_CELL is because both role="gridcell" and the <td> element map
to a single role internally, ui::AX_ROLE_CELL. So I think you should just store
a list of strings.

All of the code that parses ARIA lives in Blink currently. Perhaps this should
be a helper function in WebAXObject.

[Donn Denman, 2015/01/29 22:42:39]

Thanks for explaining this.  Moved to WebAXObject.

+      ui::AX_ROLE_ALERT_DIALOG,
+      ui::AX_ROLE_BUTTON,
+      ui::AX_ROLE_CHECK_BOX,
+      ui::AX_ROLE_DIALOG,
+      // TODO(donnd): add these missing role definitions!
+      //          ui::AX_ROLE_GRID_CELL,
+      ui::AX_ROLE_LINK,
+      ui::AX_ROLE_MARQUEE,
+      ui::AX_ROLE_MENU_ITEM,
+      ui::AX_ROLE_MENU_ITEM_CHECK_BOX,
+      ui::AX_ROLE_MENU_ITEM_RADIO,
+      //           ui::AX_ROLE_OPTION,
+      //           ui::AX_ROLE_PROGRESS_BAR,
+      //           ui::AX_ROLE_RADIO,
+      ui::AX_ROLE_SCROLL_BAR,
+      ui::AX_ROLE_SLIDER,
+      ui::AX_ROLE_SPIN_BUTTON,
+      ui::AX_ROLE_STATUS,
+      ui::AX_ROLE_TAB,
+      ui::AX_ROLE_TAB_PANEL,
+      //            ui::AX_ROLE_TEXT_BOX,
+      ui::AX_ROLE_TIMER,
+      ui::AX_ROLE_TOOLTIP,
+      ui::AX_ROLE_TREE_ITEM,
+      // Composite roles.
+      ui::AX_ROLE_COMBO_BOX,
+      ui::AX_ROLE_GRID,
+      ui::AX_ROLE_LIST_BOX,
+      ui::AX_ROLE_MENU,
+      ui::AX_ROLE_MENU_BAR,
+      ui::AX_ROLE_RADIO_GROUP,
+      ui::AX_ROLE_TAB_LIST,
+      ui::AX_ROLE_TREE,
+      ui::AX_ROLE_TREE_GRID};
+  WebNode curNode = node;
+  do {
+    if (curNode.isElementNode()) {
+      const blink::WebElement& element = curNode.toConst<blink::WebElement>();
+      VLOG(0) << "ctxs markup: " << element.createMarkup().utf8().data();
+      std::string role = element.getAttribute("role").utf8().data();

[dmazzoni, 2015/01/23 17:39:56]

The role attribute is actually allowed to be a space-delimited list of role
names; you have to parse them one at a time and take the first one that's known.
That's a good argument in favor of using WebAXObject instead of duplicating
logic here.

[Donn Denman, 2015/01/29 22:42:39]

Done in the Blink CL.  Thanks for pointing this out.

+      if (role.length() > 0) {
+        VLOG(0) << "ctxs role: " << role;
+        for (unsigned i = 0;

[dmazzoni, 2015/01/23 17:39:56]

I'd use a hash_set for this - even if we don't think performance matters because
it only happens once per tap, I think it's cleaner.

[Donn Denman, 2015/01/29 22:42:39]

Done.

+             i < sizeof(widget_and_composite_roles) / sizeof(roles[0]); i++) {
+          if (ui::ToString(widget_and_composite_roles[i]) == role)

[dmazzoni, 2015/01/23 17:39:56]

This should be case-insensitive. I'd convert the attribute value to lowercase
and then do the hash lookup.

[Donn Denman, 2015/01/29 22:42:39]

Done in the Blink CL.  Thanks for pointing this out too!

+            return true;
+        }
+      }
+    }
+    curNode = curNode.parentNode();
+  } while (!curNode.isNull());
+  return false;
+}
 #endif
 
 void RenderWidget::didHandleGestureEvent(