Disable touch highlight when the touched node has no hand-cursor.

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 1150 matching lines @@
     else
         rect.x = std::max<float>(rect.x, hitPoint.x + padding - screenWidth);
     scroll.x = rect.x;
     scroll.y = rect.y;
 
     scale = clampPageScaleFactorToLimits(scale);
     scroll = mainFrameImpl()->frameView()->windowToContents(scroll);
     scroll = clampOffsetAtScale(scroll, scale);
 }
 
+static Node* findCursorDefiningAncestor(Node* node, LocalFrame* frame)
+{
+    // Go up the tree to find the node that defines a mouse cursor style
+    while (node) {
+        // If the current node has an attached renderer, check the cursor. Otherwise, go up.

[Rick Byers, 2014/07/24 18:29:36]

I'd omit this comment - the code says this very clearly.

[mustaq, 2014/07/29 18:34:50]

Done.

+        if (node->renderer()) {
+            ECursor cursor = node->renderer()->style()->cursor();
+            if (cursor != CURSOR_AUTO || frame->eventHandler().useHandCursor(node, node->isLink()))

[Rick Byers, 2014/07/24 18:29:36]

I don't think this is right.  If a node explicitly sets the cursor to 'auto'
then that should also prevent highlight.  Eg. try this test case
http://jsbin.com/mojole/1/edit.  I think I've led you astray on this CL - I'm
sorry.  I've just done a bunch of digging and understand this better now, let's
talk offline.

[mustaq, 2014/07/29 18:34:50]

After some experiment based on yours, it seems that
node->renderer()->style()->cursor() produces not the "final" cursor but the one
defined in this node's DOM. I'll show you a case offline.

On 2014/07/24 18:29:36, Rick Byers wrote:

[mustaq, 2014/08/08 18:24:53]

After fixing the test failures caused by changes in drawing algorithm
(https://code.google.com/p/chromium/issues/detail?id=399719), I found that all
remaining test failures were caused by seemingly inconsistent value of
hit-test-result->isOverLink. I have a demo to show you offline. 

At this moment, I have reverted back to my last working logic which makes all
tests happy, and keeps the behavior above  (http://jsbin.com/mojole/1/edit)
unchanged. This is not the best solution, but we are not making this case worse
at least.

On 2014/07/29 18:34:50, mustaq wrote:

+                break;
+        }
+        node = node->parentNode();
+    }
+
+    return node;
+}
+
 static bool invokesHandCursor(Node* node, LocalFrame* frame)
 {
     if (!node || !node->renderer())
         return false;
 
     ECursor cursor = node->renderer()->style()->cursor();
     return cursor == CURSOR_POINTER
         || (cursor == CURSOR_AUTO && frame->eventHandler().useHandCursor(node, node->isLink()));
 }
 
 Node* WebViewImpl::bestTapNode(const PlatformGestureEvent& tapEvent)
 {
     TRACE_EVENT0("input", "WebViewImpl::bestTapNode");
 
     if (!m_page || !m_page->mainFrame())
         return 0;
 
-    Node* bestTouchNode = 0;
-
     // FIXME: Rely on earlier hit test instead of hit testing again.
-    GestureEventWithHitTestResults targetedEvent = m_page->deprecatedLocalMainFrame()->eventHandler().targetGestureEvent(tapEvent, true);
-    bestTouchNode = targetedEvent.hitTestResult().targetNode();
+    GestureEventWithHitTestResults targetedEvent =
+        m_page->deprecatedLocalMainFrame()->eventHandler().targetGestureEvent(tapEvent, true);
+    Node* bestTouchNode = targetedEvent.hitTestResult().targetNode();
 
     // 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
     while (bestTouchNode && !bestTouchNode->renderer())
         bestTouchNode = bestTouchNode->parentNode();
 
-    // Check if we're in the subtree of a node with a hand cursor
-    // this is the heuristic we use to determine if we show a highlight on tap
-    while (bestTouchNode && !invokesHandCursor(bestTouchNode, m_page->deprecatedLocalMainFrame()))
-        bestTouchNode = bestTouchNode->parentNode();
+    Node* cursorDefiningAncestor =
+        findCursorDefiningAncestor(bestTouchNode, m_page->deprecatedLocalMainFrame());
+    // We show a highlight on tap only when the current node shows a hand cursor
+    if (!cursorDefiningAncestor || !invokesHandCursor(cursorDefiningAncestor, m_page->deprecatedLocalMainFrame())) {
+        return 0;
+    }
 
-    if (!bestTouchNode)
-        return 0;
-
-    // We should pick the largest enclosing node with hand cursor set.
-    while (bestTouchNode->parentNode() && invokesHandCursor(bestTouchNode->parentNode(), toLocalFrame(m_page->mainFrame())))
-        bestTouchNode = bestTouchNode->parentNode();
+    // 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
+    // the next cursor-defining ancestor up in the the tree and repeat the jumps as long as the node
+    // has hand cursor set.
+    do {
+        bestTouchNode = cursorDefiningAncestor;
+        cursorDefiningAncestor = findCursorDefiningAncestor(bestTouchNode->parentNode(), m_page->deprecatedLocalMainFrame());
+    } while (cursorDefiningAncestor && invokesHandCursor(cursorDefiningAncestor, m_page->deprecatedLocalMainFrame()));
 
     return bestTouchNode;
 }
 
 void WebViewImpl::enableTapHighlightAtPoint(const PlatformGestureEvent& tapEvent)
 {
     Node* touchNode = bestTapNode(tapEvent);
 
     WillBeHeapVector<RawPtrWillBeMember<Node> > highlightNodes;
     highlightNodes.append(touchNode);
@@ skipping 3014 matching lines @@
     const PageScaleConstraints& constraints = m_pageScaleConstraintsSet.pageDefinedConstraints();
 
     if (!mainFrameImpl() || !mainFrameImpl()->frameView())
         return false;
 
     return mainFrameImpl()->frameView()->layoutSize().width() == m_size.width
         || (constraints.minimumScale == constraints.maximumScale && constraints.minimumScale != -1);
 }
 
 } // namespace blink