From f334ac54b81b2d7c3cb0bf62b02a3a4929a07ff9 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Tue, 13 Jul 2021 08:15:55 +0000 Subject: [PATCH] Bug 1715603 - part 2: Make `AutoScrollChild` not start autoscroll if a modifier key is pressed r=Gijs Now, `nsIFrame::HandleEvent` moves selection at middle mouse button down. This occurs before dispatching the event into the system event group. Therefore, `AutoScrollChild` cannot prevent it. On the other hand, Chrome does not start autoscroll when a modifier is pressed. This means that our users may not be able to use middle click with modifiers if web apps do not call `preventDefault()` as expected. So, this difference is a potential risk of web-compat. Therefore, this patch makes `AutoScrollChild` stop starting autoscroll if `Shift` key is pressed. Differential Revision: https://phabricator.services.mozilla.com/D119253 --- modules/libpref/init/all.js | 8 ++ toolkit/actors/AutoScrollChild.jsm | 35 +++++- ...rowser_click_event_during_autoscrolling.js | 106 ++++++++++++++++++ 3 files changed, 145 insertions(+), 4 deletions(-) diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 2e7675f953c7..305a4e6b2ef0 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -221,6 +221,14 @@ pref("general.config.obscure_value", 13); // for MCD .cfg files pref("general.warnOnAboutConfig", true); #endif +// Whether middle button click with a modifier key starts to autoscroll or +// does nothing. +pref("general.autoscroll.prevent_to_start.shiftKey", true); // Shift +pref("general.autoscroll.prevent_to_start.ctrlKey", false); // Control +pref("general.autoscroll.prevent_to_start.altKey", false); // Alt +pref("general.autoscroll.prevent_to_start.metaKey", false); // Command on macOS +pref("general.autoscroll.prevent_to_start.osKey", false); // Windows key on Windows or Super key on Linux + // maximum number of dated backups to keep at any time pref("browser.bookmarks.max_backups", 5); diff --git a/toolkit/actors/AutoScrollChild.jsm b/toolkit/actors/AutoScrollChild.jsm index bf832542396b..fcaf3866eee9 100644 --- a/toolkit/actors/AutoScrollChild.jsm +++ b/toolkit/actors/AutoScrollChild.jsm @@ -337,6 +337,36 @@ class AutoScrollChild extends JSWindowActorChild { this._scrollable.ownerGlobal.requestAnimationFrame(this.autoscrollLoop); } + canStartAutoScrollWith(event) { + if ( + !event.isTrusted || + event.defaultPrevented || + event.button !== 1 || + event.clickEventPrevented() + ) { + return false; + } + + for (const modifier of ["shift", "alt", "ctrl", "meta"]) { + if ( + event[modifier + "Key"] && + Services.prefs.getBoolPref( + `general.autoscroll.prevent_to_start.${modifier}Key`, + false + ) + ) { + return false; + } + } + if ( + event.getModifierState("OS") && + Services.prefs.getBoolPref("general.autoscroll.prevent_to_start.osKey") + ) { + return false; + } + return true; + } + handleEvent(event) { switch (event.type) { case "mousemove": @@ -345,10 +375,7 @@ class AutoScrollChild extends JSWindowActorChild { break; case "mousedown": if ( - event.isTrusted && - !event.defaultPrevented && - event.button === 1 && - !event.clickEventPrevented() && + this.canStartAutoScrollWith(event) && !this._scrollable && !this.isAutoscrollBlocker(event.originalTarget) ) { diff --git a/toolkit/content/tests/browser/browser_click_event_during_autoscrolling.js b/toolkit/content/tests/browser/browser_click_event_during_autoscrolling.js index 51496b4263eb..33bd71a51a34 100644 --- a/toolkit/content/tests/browser/browser_click_event_during_autoscrolling.js +++ b/toolkit/content/tests/browser/browser_click_event_during_autoscrolling.js @@ -143,6 +143,7 @@ add_task(async function() { } } + await SpecialPowers.pushPrefEnv({ set: [["middlemouse.paste", true]] }); await (async function testMouseEventsAtStartingAutoScrolling() { info( "Waiting autoscroller popup for testing mouse events at starting autoscrolling" @@ -204,6 +205,111 @@ add_task(async function() { await waitForAutoScrollEnd; })(); + if ( + // Bug 1693240: We don't support setting modifiers while posting a mouse event on Windows. + !navigator.platform.includes("Win") && + // Bug 1693237: We don't support setting modifiers on Android. + !navigator.appVersion.includes("Android") && + // In Headless mode, modifiers are not supported by this kind of APIs. + !Cc["@mozilla.org/gfx/info;1"].getService(Ci.nsIGfxInfo).isHeadless + ) { + await SpecialPowers.pushPrefEnv({ + set: [ + ["general.autoscroll.prevent_to_start.shiftKey", true], + ["general.autoscroll.prevent_to_start.altKey", true], + ["general.autoscroll.prevent_to_start.ctrlKey", true], + ["general.autoscroll.prevent_to_start.metaKey", true], + ], + }); + for (const modifier of ["Shift", "Control", "Alt", "Meta"]) { + if (modifier == "Meta" && !navigator.platform.includes("Mac")) { + continue; // Delete this after fixing bug 1232918. + } + await (async function modifiersPreventToStartAutoScrolling() { + info( + `Waiting to check not to open autoscroller popup with middle button click with ${modifier}` + ); + await promiseFlushLayoutInContent(); + let eventsInContent = new ContentEventCounter(browser, [ + "click", + "auxclick", + "mousedown", + "mouseup", + "paste", + ]); + // Ensure that the event listeners added in the content with accessing + // the remote content. + await promiseFlushLayoutInContent(); + await EventUtils.promiseNativeMouseEvent({ + type: "mousemove", + target: browser, + atCenter: true, + }); + info( + `Waiting to MozAutoScrollNoStart event for the middle button click with ${modifier}` + ); + await EventUtils.promiseNativeMouseEvent({ + type: "mousedown", + target: browser, + atCenter: true, + button: 1, // middle button + modifiers: { + altKey: modifier == "Alt", + ctrlKey: modifier == "Control", + metaKey: modifier == "Meta", + shiftKey: modifier == "Shift", + }, + }); + try { + await TestUtils.waitForCondition( + () => autoScroller?.state == "open", + `Waiting to check not to open autoscroller popup with ${modifier}`, + 100, + 10 + ); + ok( + false, + `The autoscroller popup shouldn't be opened by middle click with ${modifier}` + ); + } catch (ex) { + ok( + true, + `The autoscroller popup was not open as expected after middle click with ${modifier}` + ); + } + // In the wild, native "mouseup" event occurs after the popup is open. + await EventUtils.promiseNativeMouseEvent({ + type: "mouseup", + target: browser, + atCenter: true, + button: 1, // middle button + }); + await promiseFlushLayoutInContent(); + await promiseContentTick(); + await eventsInContent.promiseMouseEvents( + ["paste"], + `At middle clicking with ${modifier}` + ); + for (let eventType of [ + "mousedown", + "mouseup", + "click", + "auxclick", + "paste", + ]) { + is( + eventsInContent.getCountAndRemoveEventListener(eventType), + 1, + `"${eventType}" event should be fired in the content when a middle click with ${modifier}` + ); + } + info( + "Waiting autoscroller close for preparing the following tests" + ); + })(); + } + } + async function doTestMouseEventsAtStoppingAutoScrolling({ aButton = 0, aClickOutsideAutoScroller = false,