Disable gesture tap highlight on editable nodes

Source/web/WebViewImpl.cpp

 /*
  * Copyright (C) 2011, 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 1246 matching lines @@
 }
 
 Node* WebViewImpl::bestTapNode(const GestureEventWithHitTestResults& targetedTapEvent)
 {
     TRACE_EVENT0("input", "WebViewImpl::bestTapNode");
 
     if (!m_page || !m_page->mainFrame())
         return 0;
 
     Node* bestTouchNode = targetedTapEvent.hitTestResult().innerNode();
+    if (!bestTouchNode)
+        return nullptr;
 
     // We might hit something like an image map that has no renderer on it
     // Walk up the tree until we have a node with an attached renderer
     // FIXME: This wants to walk composed tree with NodeRenderingTraversal::parent().

[Rick Byers, 2015/02/20 20:37:14]

nit: mind removing this FIXME while you're here?  It was fixed but somehow the
comment didn't get removed.

[kojii, 2015/02/23 03:44:18]

Done.

-    while (bestTouchNode && !bestTouchNode->renderer())
+    while (!bestTouchNode->renderer()) {
         bestTouchNode = NodeRenderingTraversal::parent(*bestTouchNode);
+        if (!bestTouchNode)
+            return nullptr;
+    }
+
+    // Editable nodes should not be highlighted (e.g., <input>)

[hayato, 2015/02/17 10:34:56]

Not sure this is always true.
I'm afraid that this would disable highlights in other cases with false
positive.

I think you might want to add other reviewers who are familiar with how
highlight should be.

[kojii, 2015/02/17 13:08:02]

Highlights are for links, so I suppose it's quite unlikely to wrap editable
content with <a>.

I wondered the opposite case, where we may also want to disable other UA-shadow
controls, but thought focusing on reported case would be better.

I'll look around similar bugs and ask more reviewers.

[Rick Byers, 2015/02/20 20:37:14]

Highlights are for more than "links", eg. a <div> that's logically a button
should get highlighted.  But this is a heuristic, and I suspect this is a
net-improvement (although the google image search case does feel kind of like a
bug in the page - the pointer cursor is being used even when clicking will do
nothing instead of the typical affordance for something the sites is indicating
is clickable).

+    if (bestTouchNode->hasEditableStyle())
+        return nullptr;
 
     Node* cursorDefiningAncestor =
         findCursorDefiningAncestor(bestTouchNode, m_page->deprecatedLocalMainFrame());
     // We show a highlight on tap only when the current node shows a hand cursor
     if (!cursorDefiningAncestor || !showsHandCursor(cursorDefiningAncestor, m_page->deprecatedLocalMainFrame())) {
         return 0;
     }
 
     // We should pick the largest enclosing node with hand cursor set. We do this by first jumping
     // up to cursorDefiningAncestor (which is already known to have hand cursor set). Then we locate
@@ skipping 3332 matching lines @@
     return mainFrameImpl()->frameView()->layoutSize().width() == m_size.width
         || (constraints.minimumScale == constraints.maximumScale && constraints.minimumScale != -1);
 }
 
 void WebViewImpl::forceNextWebGLContextCreationToFail()
 {
     WebGLRenderingContext::forceNextWebGLContextCreationToFail();
 }
 
 } // namespace blink