Don't consider container nodes of other disambiguated nodes
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Dec 2012 01:09:58 +0000 (01:09 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Dec 2012 01:09:58 +0000 (01:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=104619

Patch by Tien-Ren Chen <trchen@chromium.org> on 2012-12-13
Reviewed by Eric Seidel.

Source/WebCore:

It is not uncommon to have a clickable <div> that contains other clickable objects.
This heuristic avoids excessive disambiguation in that case.

New unit test: WebFrameTest.DisambiguationPopupNoContainer

* page/TouchDisambiguation.cpp:
(WebCore::findGoodTouchTargets):

Source/WebKit/chromium:

Added a test to track the new disambiguation popup heuristics.

* tests/WebFrameTest.cpp:
* tests/data/disambiguation_popup_no_container.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@137686 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/page/TouchDisambiguation.cpp
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/tests/WebFrameTest.cpp
Source/WebKit/chromium/tests/data/disambiguation_popup_no_container.html [new file with mode: 0644]

index 524a3c6..33d2a84 100644 (file)
@@ -1,3 +1,18 @@
+2012-12-13  Tien-Ren Chen  <trchen@chromium.org>
+
+        Don't consider container nodes of other disambiguated nodes
+        https://bugs.webkit.org/show_bug.cgi?id=104619
+
+        Reviewed by Eric Seidel.
+
+        It is not uncommon to have a clickable <div> that contains other clickable objects.
+        This heuristic avoids excessive disambiguation in that case.
+
+        New unit test: WebFrameTest.DisambiguationPopupNoContainer
+
+        * page/TouchDisambiguation.cpp:
+        (WebCore::findGoodTouchTargets):
+
 2012-12-13  Adrienne Walker  <enne@chromium.org>
 
         Unreviewed, rolling out r137645, r137646, and r137667.
index 1060e19..8cf9d50 100644 (file)
@@ -39,6 +39,7 @@
 #include "HTMLNames.h"
 #include "HitTestResult.h"
 #include "NodeTraversal.h"
+#include "RenderBlock.h"
 #include <algorithm>
 #include <cmath>
 
@@ -100,10 +101,29 @@ void findGoodTouchTargets(const IntRect& touchBox, Frame* mainFrame, float pageS
     HitTestResult result = mainFrame->eventHandler()->hitTestResultAtPoint(contentsPoint, false, false, DontHitTestScrollbars, HitTestRequest::Active | HitTestRequest::ReadOnly, IntSize(padding, padding));
     const ListHashSet<RefPtr<Node> >& hitResults = result.rectBasedTestResult();
 
+    // Blacklist nodes that are container of disambiguated nodes.
+    // It is not uncommon to have a clickable <div> that contains other clickable objects.
+    // This heuristic avoids excessive disambiguation in that case.
+    HashSet<Node*> blackList;
+    for (ListHashSet<RefPtr<Node> >::const_iterator it = hitResults.begin(); it != hitResults.end(); ++it) {
+        RenderObject* renderer = it->get()->renderer();
+        if (!renderer)
+            continue;
+        for (RenderBlock* container = renderer->containingBlock(); container; container = container->containingBlock()) {
+            Node* containerNode = container->node();
+            if (!containerNode)
+                continue;
+            if (!blackList.add(containerNode).isNewEntry)
+                break;
+        }
+    }
+
     HashMap<Node*, TouchTargetData> touchTargets;
     float bestScore = 0;
     for (ListHashSet<RefPtr<Node> >::const_iterator it = hitResults.begin(); it != hitResults.end(); ++it) {
         for (Node* node = it->get(); node; node = node->parentNode()) {
+            if (blackList.contains(it->get()))
+                continue;
             if (node->isDocumentNode() || node->hasTagName(HTMLNames::htmlTag) || node->hasTagName(HTMLNames::bodyTag))
                 break;
             if (node->willRespondToMouseClickEvents()) {
index 82dc28f..fcd55ee 100644 (file)
@@ -1,3 +1,15 @@
+2012-12-13  Tien-Ren Chen  <trchen@chromium.org>
+
+        Don't consider container nodes of other disambiguated nodes
+        https://bugs.webkit.org/show_bug.cgi?id=104619
+
+        Reviewed by Eric Seidel.
+
+        Added a test to track the new disambiguation popup heuristics.
+
+        * tests/WebFrameTest.cpp:
+        * tests/data/disambiguation_popup_no_container.html: Added.
+
 2012-12-13  James Robinson  <jamesr@chromium.org>
 
         [chromium] Expose a WebLayerTreeView getter on WebWidget to make it easier for the embedder to interface with the compositor
index 29e0f0b..26020f7 100644 (file)
@@ -1542,7 +1542,7 @@ static WebGestureEvent fatTap(int x, int y)
     return event;
 }
 
-TEST_F(WebFrameTest, DisambiguationPopupTest)
+TEST_F(WebFrameTest, DisambiguationPopup)
 {
     registerMockedHttpURLLoad("disambiguation_popup.html");
 
@@ -1586,6 +1586,23 @@ TEST_F(WebFrameTest, DisambiguationPopupTest)
 
 }
 
+TEST_F(WebFrameTest, DisambiguationPopupNoContainer)
+{
+    registerMockedHttpURLLoad("disambiguation_popup_no_container.html");
+
+    DisambiguationPopupTestWebViewClient client;
+
+    // Make sure we initialize to minimum scale, even if the window size
+    // only becomes available after the load begins.
+    WebViewImpl* webViewImpl = static_cast<WebViewImpl*>(FrameTestHelpers::createWebViewAndLoad(m_baseURL + "disambiguation_popup_no_container.html", true, 0, &client));
+    webViewImpl->resize(WebSize(1000, 1000));
+    webViewImpl->layout();
+
+    client.resetTriggered();
+    webViewImpl->handleInputEvent(fatTap(50, 50));
+    EXPECT_FALSE(client.triggered());
+}
+
 class TestSubstituteDataWebFrameClient : public WebFrameClient {
 public:
     TestSubstituteDataWebFrameClient()
diff --git a/Source/WebKit/chromium/tests/data/disambiguation_popup_no_container.html b/Source/WebKit/chromium/tests/data/disambiguation_popup_no_container.html
new file mode 100644 (file)
index 0000000..7d2af3e
--- /dev/null
@@ -0,0 +1,25 @@
+<html>
+<head>
+<title>Disambiguation Popup Test</title>
+<style type="text/css">
+.outer-div {
+    display:block;
+    width:200px;
+    height:200px;
+    margin:0px;
+    padding:50px;
+    background-color:#ffcccc;
+}
+.inner-link {
+    display:block;
+    width:200px;
+    height:200px;
+    margin:0px;
+    background-color:#ccffcc;
+}
+</style>
+</head>
+<body style="margin:0px;">
+<div class="outer-div" onclick=";"><a href="#" class="inner-link"></a></a>
+</body>
+</html>