Bug 1716883 - fix autoscroll behaviour over various SVG elements, r=masayuki

This avoids starting autoscroll on SVG links, and on HTML links that
contain SVG, by sharing the code that browser's ClickHandlerChild uses
to detect links into BrowserUtils, and using that from AutoScrollChild
to determine if the click happened on top of a link. It also adds a
test so we avoid regressing this in future.

When running the test, I noticed an error from ClickHandlerParent
when the browser for which we receive a click has gone away, and
fixed it by adding a nullcheck and early return.

Differential Revision: https://phabricator.services.mozilla.com/D120024

browser/actors/ClickHandlerChild.jsm

@@ -23,6 +23,12 @@ ChromeUtils.defineModuleGetter(
   "resource://gre/modules/E10SUtils.jsm"
 );
 
+ChromeUtils.defineModuleGetter(
+  this,
+  "BrowserUtils",
+  "resource://gre/modules/BrowserUtils.jsm"
+);
+
 class ClickHandlerChild extends JSWindowActorChild {
   handleEvent(event) {
     if (
@@ -59,7 +65,9 @@ class ClickHandlerChild extends JSWindowActorChild {
       }
     }
 
-    let [href, node, principal] = this._hrefAndLinkNodeForClickEvent(event);
+    let [href, node, principal] = BrowserUtils.hrefAndLinkNodeForClickEvent(
+      event
+    );
 
     let csp = ownerDoc.csp;
     if (csp) {
@@ -133,65 +141,4 @@ class ClickHandlerChild extends JSWindowActorChild {
       this.sendAsyncMessage("Content:Click", json);
     }
   }
-
-  /**
-   * Extracts linkNode and href for the current click target.
-   *
-   * @param event
-   *        The click event.
-   * @return [href, linkNode, linkPrincipal].
-   *
-   * @note linkNode will be null if the click wasn't on an anchor
-   *       element. This includes SVG links, because callers expect |node|
-   *       to behave like an <a> element, which SVG links (XLink) don't.
-   */
-  _hrefAndLinkNodeForClickEvent(event) {
-    let content = this.contentWindow;
-    function isHTMLLink(aNode) {
-      // Be consistent with what nsContextMenu.js does.
-      return (
-        (aNode instanceof content.HTMLAnchorElement && aNode.href) ||
-        (aNode instanceof content.HTMLAreaElement && aNode.href) ||
-        aNode instanceof content.HTMLLinkElement
-      );
-    }
-
-    let node = event.composedTarget;
-    while (node && !isHTMLLink(node)) {
-      node = node.flattenedTreeParentNode;
-    }
-
-    if (node) {
-      return [node.href, node, node.ownerDocument.nodePrincipal];
-    }
-
-    // If there is no linkNode, try simple XLink.
-    let href, baseURI;
-    node = event.composedTarget;
-    while (node && !href) {
-      if (
-        node.nodeType == content.Node.ELEMENT_NODE &&
-        (node.localName == "a" ||
-          node.namespaceURI == "http://www.w3.org/1998/Math/MathML")
-      ) {
-        href =
-          node.getAttribute("href") ||
-          node.getAttributeNS("http://www.w3.org/1999/xlink", "href");
-        if (href) {
-          baseURI = node.ownerDocument.baseURIObject;
-          break;
-        }
-      }
-      node = node.flattenedTreeParentNode;
-    }
-
-    // In case of XLink, we don't return the node we got href from since
-    // callers expect <a>-like elements.
-    // Note: makeURI() will throw if aUri is not a valid URI.
-    return [
-      href ? Services.io.newURI(href, null, baseURI).spec : null,
-      null,
-      node && node.ownerDocument.nodePrincipal,
-    ];
-  }
 }

browser/actors/ClickHandlerParent.jsm

@@ -55,6 +55,11 @@ class ClickHandlerParent extends JSWindowActorParent {
     // This is heavily based on contentAreaClick from browser.js (Bug 903016)
     // The data is set up in a way to look like an Event.
     let browser = this.manager.browsingContext.top.embedderElement;
+    if (!browser) {
+      // Can be null if the tab disappeared by the time we got the message.
+      // Just bail.
+      return;
+    }
     let window = browser.ownerGlobal;
 
     if (!data.href) {

toolkit/actors/AutoScrollChild.jsm

@@ -3,9 +3,15 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+var EXPORTED_SYMBOLS = ["AutoScrollChild"];
+
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
-var EXPORTED_SYMBOLS = ["AutoScrollChild"];
+ChromeUtils.defineModuleGetter(
+  this,
+  "BrowserUtils",
+  "resource://gre/modules/BrowserUtils.jsm"
+);
 
 class AutoScrollChild extends JSWindowActorChild {
   constructor() {
@@ -25,11 +31,12 @@ class AutoScrollChild extends JSWindowActorChild {
     this.autoscrollLoop = this.autoscrollLoop.bind(this);
   }
 
-  isAutoscrollBlocker(node) {
+  isAutoscrollBlocker(event) {
     let mmPaste = Services.prefs.getBoolPref("middlemouse.paste");
     let mmScrollbarPosition = Services.prefs.getBoolPref(
       "middlemouse.scrollbarPosition"
     );
+    let node = event.originalTarget;
     let content = node.ownerGlobal;
 
     // If the node is in editable document or content, we don't want to start
@@ -45,35 +52,28 @@ class AutoScrollChild extends JSWindowActorChild {
       }
     }
 
-    while (node) {
+    // Don't start if we're on a link.
-      if (
+    let [href] = BrowserUtils.hrefAndLinkNodeForClickEvent(event);
-        (node instanceof content.HTMLAnchorElement ||
+    if (href) {
-          node instanceof content.HTMLAreaElement) &&
-        node.hasAttribute("href")
-      ) {
       return true;
     }
 
+    // Or if we're pasting into an input field of sorts.
     if (
       mmPaste &&
-        (node instanceof content.HTMLInputElement ||
+      node.closest("input,textarea")?.constructor.name.startsWith("HTML")
-          node instanceof content.HTMLTextAreaElement)
     ) {
       return true;
     }
 
+    // Or if we're on a scrollbar or XUL <tree>
     if (
-        node instanceof content.XULElement &&
+      (mmScrollbarPosition &&
-        ((mmScrollbarPosition &&
+        node.closest("scrollbar,scrollcorner") instanceof content.XULElement) ||
-          (node.localName == "scrollbar" ||
+      node.closest("treechildren") instanceof content.XULElement
-            node.localName == "scrollcorner")) ||
-          node.localName == "treechildren")
     ) {
       return true;
     }
-
-      node = node.parentNode;
-    }
     return false;
   }
 
@@ -377,7 +377,7 @@ class AutoScrollChild extends JSWindowActorChild {
         if (
           this.canStartAutoScrollWith(event) &&
           !this._scrollable &&
-          !this.isAutoscrollBlocker(event.originalTarget)
+          !this.isAutoscrollBlocker(event)
         ) {
           this.startScroll(event);
         }

toolkit/content/tests/browser/browser.ini

@@ -23,6 +23,7 @@ support-files =
 [browser_autoscroll_disabled.js]
 skip-if = true # Bug 1312652
 [browser_autoscroll_disabled_on_editable_content.js]
+[browser_autoscroll_disabled_on_links.js]
 [browser_delay_autoplay_media.js]
 tags = audiochannel
 skip-if = (os == "win" && processor == "aarch64") # aarch64 due to 1536573

toolkit/content/tests/browser/browser_autoscroll_disabled_on_links.js

@@ -0,0 +1,125 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+add_task(async function test_autoscroll_links() {
+  await SpecialPowers.pushPrefEnv({
+    set: [
+      ["general.autoScroll", true],
+      ["middlemouse.contentLoadURL", false],
+      ["test.events.async.enabled", false],
+    ],
+  });
+
+  let autoScroller;
+  function onPopupShown(aEvent) {
+    if (aEvent.originalTarget.id != "autoscroller") {
+      return false;
+    }
+    autoScroller = aEvent.originalTarget;
+    return true;
+  }
+  window.addEventListener("popupshown", onPopupShown, { capture: true });
+  registerCleanupFunction(() => {
+    window.removeEventListener("popupshown", onPopupShown, { capture: true });
+  });
+  function popupIsNotClosed() {
+    return autoScroller && autoScroller.state != "closed";
+  }
+
+  async function promiseNativeMouseMiddleButtonDown(aBrowser) {
+    await EventUtils.promiseNativeMouseEvent({
+      type: "mousemove",
+      target: aBrowser,
+      atCenter: true,
+    });
+    return EventUtils.promiseNativeMouseEvent({
+      type: "mousedown",
+      target: aBrowser,
+      atCenter: true,
+      button: 1, // middle button
+    });
+  }
+  async function promiseNativeMouseMiddleButtonUp(aBrowser) {
+    return EventUtils.promiseNativeMouseEvent({
+      type: "mouseup",
+      target: aBrowser,
+      atCenter: true,
+      button: 1, // middle button
+    });
+  }
+  function promiseWaitForAutoScrollerClosed() {
+    if (!autoScroller || autoScroller.state == "closed") {
+      return Promise.resolve();
+    }
+    let result = BrowserTestUtils.waitForEvent(
+      autoScroller,
+      "popuphidden",
+      { capture: true },
+      () => {
+        return true;
+      }
+    );
+    EventUtils.synthesizeKey("KEY_Escape");
+    return result;
+  }
+
+  async function testMarkup(markup) {
+    return BrowserTestUtils.withNewTab(
+      "https://example.com/browser/toolkit/content/tests/browser/file_empty.html",
+      async function(browser) {
+        await SpecialPowers.spawn(browser, [markup], html => {
+          // eslint-disable-next-line no-unsanitized/property
+          content.document.body.innerHTML = html;
+          content.document.documentElement.scrollTop = 1;
+          content.document.documentElement.scrollTop; // Flush layout.
+        });
+        await promiseNativeMouseMiddleButtonDown(browser);
+        try {
+          await TestUtils.waitForCondition(
+            popupIsNotClosed,
+            "Wait for timeout of popup",
+            100,
+            10
+          );
+          ok(false, "Autoscroll shouldn't be started on " + markup);
+        } catch (e) {
+          ok(
+            typeof e == "string" && e.includes(" - timed out after 10 tries."),
+            `Autoscroll shouldn't be started on ${markup} (${
+              typeof e == "string" ? e : e.message
+            })`
+          );
+        } finally {
+          await promiseNativeMouseMiddleButtonUp(browser);
+          let waitForAutoScrollEnd = promiseWaitForAutoScrollerClosed();
+          await waitForAutoScrollEnd;
+        }
+      }
+    );
+  }
+
+  await testMarkup(
+    '<a href="https://example.com/" style="display: block; position: absolute; height:100%; width:100%; background: aqua">Click me</a>'
+  );
+
+  await testMarkup(`
+    <svg viewbox="0 0 100 100" style="display: block; height: 100%; width: 100%;">
+      <a href="https://example.com/">
+        <rect height=100 width=100 fill=blue />
+      </a>
+    </svg>`);
+
+  await testMarkup(`
+    <a href="https://example.com/">
+      <svg viewbox="0 0 100 100" style="display: block; height: 100%; width: 100%;">
+        <use href="#x"/>
+      </svg>
+    </a>
+
+    <svg viewbox="0 0 100 100" style="display: none">
+      <rect id="x" height=100 width=100 fill=green />
+    </svg>
+    `);
+});

toolkit/modules/BrowserUtils.jsm

@@ -140,6 +140,71 @@ var BrowserUtils = {
       "moz-extension" == scheme
     );
   },
+
+  /**
+   * Extracts linkNode and href for a click event.
+   *
+   * @param event
+   *        The click event.
+   * @return [href, linkNode, linkPrincipal].
+   *
+   * @note linkNode will be null if the click wasn't on an anchor
+   *       element. This includes SVG links, because callers expect |node|
+   *       to behave like an <a> element, which SVG links (XLink) don't.
+   */
+  hrefAndLinkNodeForClickEvent(event) {
+    // We should get a window off the event, and bail if not:
+    let content = event.view || event.composedTarget?.ownerGlobal;
+    if (!content?.HTMLAnchorElement) {
+      return null;
+    }
+    function isHTMLLink(aNode) {
+      // Be consistent with what nsContextMenu.js does.
+      return (
+        (aNode instanceof content.HTMLAnchorElement && aNode.href) ||
+        (aNode instanceof content.HTMLAreaElement && aNode.href) ||
+        aNode instanceof content.HTMLLinkElement
+      );
+    }
+
+    let node = event.composedTarget;
+    while (node && !isHTMLLink(node)) {
+      node = node.flattenedTreeParentNode;
+    }
+
+    if (node) {
+      return [node.href, node, node.ownerDocument.nodePrincipal];
+    }
+
+    // If there is no linkNode, try simple XLink.
+    let href, baseURI;
+    node = event.composedTarget;
+    while (node && !href) {
+      if (
+        node.nodeType == content.Node.ELEMENT_NODE &&
+        (node.localName == "a" ||
+          node.namespaceURI == "http://www.w3.org/1998/Math/MathML")
+      ) {
+        href =
+          node.getAttribute("href") ||
+          node.getAttributeNS("http://www.w3.org/1999/xlink", "href");
+        if (href) {
+          baseURI = node.ownerDocument.baseURIObject;
+          break;
+        }
+      }
+      node = node.flattenedTreeParentNode;
+    }
+
+    // In case of XLink, we don't return the node we got href from since
+    // callers expect <a>-like elements.
+    // Note: makeURI() will throw if aUri is not a valid URI.
+    return [
+      href ? Services.io.newURI(href, null, baseURI).spec : null,
+      null,
+      node && node.ownerDocument.nodePrincipal,
+    ];
+  },
 };
 
 XPCOMUtils.defineLazyPreferenceGetter(