Bug 1191277 - Ensure that we don't find clusters of clickable elements when there is no possible way for the heuristic to actually target those elements. r=domivinc
authorKartikaya Gupta <kgupta@mozilla.com>
Sat, 15 Aug 2015 11:15:29 -0400
changeset 257954 51eebda696d4d9a2e03808b813daa3151513e9f4
parent 257953 4612c09141ae63f93b7d4dc042a18f521692f661
child 257955 f8dd6bbde6ce8165f752aa4edfe2abaa40edf8be
push id29238
push userryanvm@gmail.com
push dateMon, 17 Aug 2015 13:06:57 +0000
treeherdermozilla-central@a6eeb28458fd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdomivinc
bugs1191277
milestone43.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1191277 - Ensure that we don't find clusters of clickable elements when there is no possible way for the heuristic to actually target those elements. r=domivinc
layout/base/PositionedEventTargeting.cpp
layout/base/tests/test_event_target_radius.html
--- a/layout/base/PositionedEventTargeting.cpp
+++ b/layout/base/PositionedEventTargeting.cpp
@@ -530,16 +530,21 @@ FindFrameTargetedByInputEvent(WidgetGUIE
   nsIContent* clickableAncestor = nullptr;
   if (target) {
     clickableAncestor = GetClickableAncestor(target, nsGkAtoms::body);
     if (clickableAncestor) {
       if (!IsElementClickableAndReadable(target, aEvent, prefs)) {
         aEvent->AsMouseEventBase()->hitCluster = true;
       }
       PET_LOG("Target %p is clickable\n", target);
+      // If the target that was directly hit has a clickable ancestor, that
+      // means it too is clickable. And since it is the same as or a descendant
+      // of clickableAncestor, it should become the root for the GetClosest
+      // search.
+      clickableAncestor = target->GetContent();
     }
   }
 
   // Do not modify targeting for actual mouse hardware; only for mouse
   // events generated by touch-screen hardware.
   if (aEvent->mClass == eMouseEventClass &&
       prefs->mTouchOnly &&
       aEvent->AsMouseEvent()->inputSource !=
--- a/layout/base/tests/test_event_target_radius.html
+++ b/layout/base/tests/test_event_target_radius.html
@@ -34,16 +34,17 @@ https://bugzilla.mozilla.org/show_bug.cg
   <div class="target" style="transform:translate(-80px,0);" id="t4" onmousedown="x=1" hidden></div>
 
   <div class="target" style="left:0; z-index:1" id="t5_left" onmousedown="x=1" hidden></div>
   <div class="target" style="left:106px;" id="t5_right" onmousedown="x=1" hidden></div>
   <div class="target" style="left:0; top:210px;" id="t5_below" onmousedown="x=1" hidden></div>
 
   <div class="target" id="t6" onmousedown="x=1" hidden>
     <div id="t6_inner" style="position:absolute; left:-40px; top:20px; width:60px; height:60px; background:yellow;"></div>
+    <div id="t6_inner_clickable" style="position:absolute; left:-40px; top: 80px; width: 60px; height: 5px; background:red" onmousedown="x=1"></div>
   </div>
   <div id="t6_outer" style="position:absolute; left:160px; top:120px; width:60px; height:60px; background:green;" onmousedown="x=1" hidden></div>
 
   <div class="target" id="t7" onmousedown="x=1" hidden></div>
   <div class="target" id="t7_over" hidden></div>
 
   <div id="t8" contenteditable="true" class="target" hidden></div>
 
@@ -190,16 +191,26 @@ function test3() {
   setShowing("t6", true);
   setShowing("t6_outer", true);
   testMouseClick("t6_inner", -1, 10, "t6_inner",
     "inner element is clickable because its parent is, even when it sticks outside parent");
   testMouseClick("t6_inner", 39, -1, "t6_inner",
     "when outside both inner and parent, but in range of both, the inner is selected");
   testMouseClick("t6_inner", 45, -1, "t6_inner",
     "clicking in clickable parent close to inner activates inner, not parent");
+  testMouseClick("t6_inner_clickable", 1, -1, "t6_inner",
+    "clicking on inner doesn't get redirected to inner_clickable because they are both clickable");
+  testMouseClick("t6_inner_clickable", 1, 1, "t6_inner_clickable",
+    "clicking on inner_clickable doesn't get redirected to inner because they are both clickable");
+  testMouseClick("t6_inner_clickable", 45, -1, "t6_inner",
+    "clicking on inner while backed by its parent still doesn't get redirected to inner_clickable");
+  testMouseClick("t6_inner_clickable", 45, 1, "t6_inner_clickable",
+    "clicking on inner_clickable while backed by its parent still doesn't get redirected to inner");
+  testMouseClick("t6_inner_clickable", 45, 6, "t6_inner_clickable",
+    "clicking on parent near inner_clickable gets redirected to inner_clickable rather than inner because it is closer");
   ok(13*mm < 80, "no point inside t6 that's not within radius of t6_inner; adjust layout of t6/inner/outer as needed");
   testMouseClick("t6_outer", -40 + 13*mm, -1, "t6",
     "clicking in clickable container close to outer activates parent, not outer");
   testMouseClick("t6_outer", 1, 1, "t6_outer",
     "clicking directly on the outer activates it");
   setShowing("t6", false);
   setShowing("t6_outer", false);