Bug 1792110 - Update the common ancestor finding logic for mouseup with mousedown r=edgar

The existing function doesn't not work for cases like when mousedown and mouseup were happened in the same hierarchy but the mousedown target was a different interactive element than the mouseup target. This could be easily accomplished by setting `pointer-events: none` or `display: none` to the `active` pseudo-class of the mousedown target. This patch fixes this by when the logic tries to find the event target tree of the mousedown target, it continues building up (by recursively looking up the parents) the tree instead of stopping at the first interactive element. Differential Revision: https://phabricator.services.mozilla.com/D178844
2024-10-09 03:15:11 +00:00 · 2023-06-09 13:25:04 +00:00 · 2023-06-09 13:25:04 +00:00 · 19feafd6fd
commit 19feafd6fd
parent db0d115d6a
7 changed files with 177 additions and 67 deletions
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@ -2934,54 +2934,6 @@ int32_t nsContentUtils::ComparePoints_Deprecated(
  return *child1Index < aOffset2 ? -1 : 1;
 }

-// static
-nsINode* nsContentUtils::GetCommonAncestorUnderInteractiveContent(
-    nsINode* aNode1, nsINode* aNode2) {
-  if (!aNode1 || !aNode2) {
-    return nullptr;
-  }
-
-  if (aNode1 == aNode2) {
-    return aNode1;
-  }
-
-  // Build the chain of parents
-  AutoTArray<nsINode*, 30> parents1;
-  do {
-    parents1.AppendElement(aNode1);
-    if (aNode1->IsElement() &&
-        aNode1->AsElement()->IsInteractiveHTMLContent()) {
-      break;
-    }
-    aNode1 = aNode1->GetFlattenedTreeParentNode();
-  } while (aNode1);
-
-  AutoTArray<nsINode*, 30> parents2;
-  do {
-    parents2.AppendElement(aNode2);
-    if (aNode2->IsElement() &&
-        aNode2->AsElement()->IsInteractiveHTMLContent()) {
-      break;
-    }
-    aNode2 = aNode2->GetFlattenedTreeParentNode();
-  } while (aNode2);
-
-  // Find where the parent chain differs
-  uint32_t pos1 = parents1.Length();
-  uint32_t pos2 = parents2.Length();
-  nsINode* parent = nullptr;
-  for (uint32_t len = std::min(pos1, pos2); len > 0; --len) {
-    nsINode* child1 = parents1.ElementAt(--pos1);
-    nsINode* child2 = parents2.ElementAt(--pos2);
-    if (child1 != child2) {
-      break;
-    }
-    parent = child1;
-  }
-
-  return parent;
-}
-
 /* static */
 BrowserParent* nsContentUtils::GetCommonBrowserParentAncestor(
    BrowserParent* aBrowserParent1, BrowserParent* aBrowserParent2) {
--- a/dom/base/nsContentUtils.h
+++ b/dom/base/nsContentUtils.h
@ -526,15 +526,6 @@ class nsContentUtils {
  static Element* GetCommonFlattenedTreeAncestorForStyle(Element* aElement1,
                                                         Element* aElement2);

-  /**
-   * Returns the common ancestor under interactive content, if any.
-   * If neither one has interactive content as ancestor, common ancestor will be
-   * returned. If only one has interactive content as ancestor, null will be
-   * returned. If the nodes are the same, that node is returned.
-   */
-  static nsINode* GetCommonAncestorUnderInteractiveContent(nsINode* aNode1,
-                                                           nsINode* aNode2);
-
  /**
   * Returns the common BrowserParent ancestor, if any, for two given
   * BrowserParent.
--- a/dom/events/EventStateManager.cpp
+++ b/dom/events/EventStateManager.cpp
@ -73,6 +73,7 @@
 #include "nsIWidget.h"
 #include "nsLiteralString.h"
 #include "nsPresContext.h"
+#include "nsTArray.h"
 #include "nsGkAtoms.h"
 #include "nsIFormControl.h"
 #include "nsComboboxControlFrame.h"
@ -168,6 +169,52 @@ static UniquePtr<WidgetMouseEvent> CreateMouseOrPointerWidgetEvent(
    WidgetMouseEvent* aMouseEvent, EventMessage aMessage,
    EventTarget* aRelatedTarget);

+/**
+ * Returns the common ancestor for mouseup purpose, given the
+ * current mouseup target and the previous mousedown target.
+ */
+static nsINode* GetCommonAncestorForMouseUp(nsINode* aCurrentMouseUpTarget,
+                                            nsINode* aLastMouseDownTarget) {
+  if (!aCurrentMouseUpTarget || !aLastMouseDownTarget) {
+    return nullptr;
+  }
+
+  if (aCurrentMouseUpTarget == aLastMouseDownTarget) {
+    return aCurrentMouseUpTarget;
+  }
+
+  // Build the chain of parents
+  AutoTArray<nsINode*, 30> parents1;
+  do {
+    parents1.AppendElement(aCurrentMouseUpTarget);
+    aCurrentMouseUpTarget = aCurrentMouseUpTarget->GetFlattenedTreeParentNode();
+  } while (aCurrentMouseUpTarget);
+
+  AutoTArray<nsINode*, 30> parents2;
+  do {
+    parents2.AppendElement(aLastMouseDownTarget);
+    if (aLastMouseDownTarget == parents1.LastElement()) {
+      break;
+    }
+    aLastMouseDownTarget = aLastMouseDownTarget->GetFlattenedTreeParentNode();
+  } while (aLastMouseDownTarget);
+
+  // Find where the parent chain differs
+  uint32_t pos1 = parents1.Length();
+  uint32_t pos2 = parents2.Length();
+  nsINode* parent = nullptr;
+  for (uint32_t len = std::min(pos1, pos2); len > 0; --len) {
+    nsINode* child1 = parents1.ElementAt(--pos1);
+    nsINode* child2 = parents2.ElementAt(--pos2);
+    if (child1 != child2) {
+      break;
+    }
+    parent = child1;
+  }
+
+  return parent;
+}
+
 /******************************************************************/
 /* mozilla::UITimerCallback                                       */
 /******************************************************************/
@ -5176,8 +5223,8 @@ nsresult EventStateManager::SetClickCount(WidgetMouseEvent* aEvent,
      } else if (aEvent->mMessage == eMouseUp) {
        aEvent->mClickTarget =
            !aEvent->mClickEventPrevented
-                ? nsContentUtils::GetCommonAncestorUnderInteractiveContent(
-                      mouseContent, mLastLeftMouseDownContent)
+                ? GetCommonAncestorForMouseUp(mouseContent,
+                                              mLastLeftMouseDownContent)
                : nullptr;
        if (aEvent->mClickTarget) {
          aEvent->mClickCount = mLClickCount;
@ -5196,8 +5243,8 @@ nsresult EventStateManager::SetClickCount(WidgetMouseEvent* aEvent,
      } else if (aEvent->mMessage == eMouseUp) {
        aEvent->mClickTarget =
            !aEvent->mClickEventPrevented
-                ? nsContentUtils::GetCommonAncestorUnderInteractiveContent(
-                      mouseContent, mLastMiddleMouseDownContent)
+                ? GetCommonAncestorForMouseUp(mouseContent,
+                                              mLastMiddleMouseDownContent)
                : nullptr;
        if (aEvent->mClickTarget) {
          aEvent->mClickCount = mMClickCount;
@ -5216,8 +5263,8 @@ nsresult EventStateManager::SetClickCount(WidgetMouseEvent* aEvent,
      } else if (aEvent->mMessage == eMouseUp) {
        aEvent->mClickTarget =
            !aEvent->mClickEventPrevented
-                ? nsContentUtils::GetCommonAncestorUnderInteractiveContent(
-                      mouseContent, mLastRightMouseDownContent)
+                ? GetCommonAncestorForMouseUp(mouseContent,
+                                              mLastRightMouseDownContent)
                : nullptr;
        if (aEvent->mClickTarget) {
          aEvent->mClickCount = mRClickCount;
--- a/testing/web-platform/meta/uievents/click/click_event_target_siblings.html.ini
+++ b/testing/web-platform/meta/uievents/click/click_event_target_siblings.html.ini
@ -1,4 +0,0 @@
-[click_event_target_siblings.html]
-  [Click targets the nearest common ancestor]
-    expected: FAIL
-
--- a/testing/web-platform/tests/pointerevents/pointerevent_bubble_ancestors_none.html
+++ b/testing/web-platform/tests/pointerevents/pointerevent_bubble_ancestors_none.html
@ -0,0 +1,34 @@
+<!doctype html>
+<html>
+<head>
+<title>PointerEvent: Events still bubble to ancestors with pointer-events: none </title>
+<meta name="viewport" content="width=device-width">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+<style>
+#wrapper:active {
+    pointer-events: none;
+}
+</style>
+</head>
+<body>
+<div id="parent">
+  <div id="wrapper">
+    <button>click me</button>
+  </div>
+</div>
+
+<script>
+promise_test(function() {
+  const parentClickedPromise = new Promise(r => {
+    document.getElementById("parent").addEventListener("click", r);
+  });
+
+  const click = test_driver.click(document.querySelector("button"));
+
+  return Promise.all([click, parentClickedPromise]);
+})
+</script>
+</body>
--- a/testing/web-platform/tests/pointerevents/pointerevent_bubble_display_none.html
+++ b/testing/web-platform/tests/pointerevents/pointerevent_bubble_display_none.html
@ -0,0 +1,40 @@
+<!doctype html>
+<html>
+<head>
+<title>PointerEvent: Events still bubble to ancestors with display: none </title>
+<meta name="viewport" content="width=device-width">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+<style>
+#wrapper:active {
+  display: none;
+}
+
+#parent {
+  width: 100px;
+  height: 100px;
+  background-color: green;
+}
+</style>
+</head>
+<body>
+<div id="parent">
+  <div id="wrapper">
+    <button>click me</button>
+  </div>
+</div>
+
+<script>
+promise_test(function() {
+  const parentClickedPromise = new Promise(r => {
+    document.getElementById("parent").addEventListener("click", r);
+  });
+
+  const click = test_driver.click(document.querySelector("button"));
+
+  return Promise.all([click, parentClickedPromise]);
+})
+</script>
+</body>
--- a/testing/web-platform/tests/pointerevents/pointerevent_bubble_mousedown_mouseup_different_target.html
+++ b/testing/web-platform/tests/pointerevents/pointerevent_bubble_mousedown_mouseup_different_target.html
@ -0,0 +1,50 @@
+<!doctype html>
+<html>
+<head>
+<title>PointerEvent: Events still bubble to ancestors with mousedown causes mouseup to be a different target</title>
+<meta name="viewport" content="width=device-width">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+<script src="/resources/testdriver-actions.js"></script>
+<style>
+#wrapper {
+  display: none;
+}
+
+#parent {
+  width: 100px;
+  height: 100px;
+  background-color: green;
+}
+
+button {
+  width: 200px;
+  height: 200px;
+}
+</style>
+</head>
+<body>
+<div id="parent">
+  <div id="wrapper">
+    <button>click me</button>
+  </div>
+</div>
+
+<script>
+promise_test(function() {
+  const parentClickedPromise = new Promise(r => {
+    document.getElementById("parent").addEventListener("click", r);
+  });
+
+  document.getElementById("parent").addEventListener("mousedown", function() {
+    document.getElementById("wrapper").style.display = "block";
+  });
+
+  const click = test_driver.click(document.getElementById("parent"));
+
+  return Promise.all([click, parentClickedPromise]);
+})
+</script>
+</body>