mirror of
https://github.com/mozilla/gecko-dev.git
synced 2024-10-16 06:45:42 +00:00
Bug 1742801 - do not consume the user gesture from ClickHandlerChild if ClickHandlerParent will ignore the click anyway, r=edgar
This commit does a couple of things: - move whereToOpenLink and getRootEvent implementations into BrowserUtils, so they can be used from the child process. - forward callers in utilityOverlay.js to the BrowserUtils ones (bug 1742889 will get rid of the forwarding and update all the callers; we might be able to get this and bug 1739929 into beta if risk is low enough, and touching a bunch of extra files really doesn't help with that) - move the lazy-load of BrowserUtils from browser.js to utilityOverlay.js This is safe because everywhere that loads browser.js also loads utilityOverlay.js. It's needed because there are some places that use utilityOverlay.js but not browser.js, and so now they need access to BrowserUtils.jsm. - use whereToOpenLink to determine if we should avoid consuming the transient user gesture activation in the child click handling code. - add an automated test based on the testcase in the bug. When working on this, I initially put the check using whereToOpenLink in the toplevel of the function, and then when I ran places test to check that I hadn't broken any places consumers of whereToOpenLink or getRootEvent, realized that I had broken `browser_markPageAsFollowedLink.js`, because it relies on "normal" (ie no modifier key, left button) link clicks making it to ClickHandlerParent.jsm . I filed bug 1742894 about this. I've not tried to fix that here, instead I've tried to ensure that paths through this function are as untouched as possible while still fixing bug 1739929 and bug 1742801. Differential Revision: https://phabricator.services.mozilla.com/D132102
This commit is contained in:
Gijs Kruitbosch
parent
55f1b5646f
commit
f7af2c33b1
@ -57,13 +57,6 @@ class ClickHandlerChild extends JSWindowActorChild {
|
||||
return;
|
||||
}
|
||||
|
||||
// For untrusted events, require a valid transient user gesture activation.
|
||||
// consumeTransientUserGestureActivation returns false if there isn't one,
|
||||
// and consumes it otherwise.
|
||||
if (!event.isTrusted && !ownerDoc.consumeTransientUserGestureActivation()) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Handle click events from about pages
|
||||
if (event.button == 0) {
|
||||
if (ownerDoc.documentURI.startsWith("about:blocked")) {
|
||||
@ -71,6 +64,11 @@ class ClickHandlerChild extends JSWindowActorChild {
|
||||
}
|
||||
}
|
||||
|
||||
// For untrusted events, require a valid transient user gesture activation.
|
||||
if (!event.isTrusted && !ownerDoc.hasValidTransientUserGestureActivation) {
|
||||
return;
|
||||
}
|
||||
|
||||
let [href, node, principal] = BrowserUtils.hrefAndLinkNodeForClickEvent(
|
||||
event
|
||||
);
|
||||
@ -119,6 +117,26 @@ class ClickHandlerChild extends JSWindowActorChild {
|
||||
return;
|
||||
}
|
||||
|
||||
if (
|
||||
!event.isTrusted &&
|
||||
BrowserUtils.whereToOpenLink(event) != "current"
|
||||
) {
|
||||
// If we'll open the link, we want to consume the user gesture
|
||||
// activation to ensure that we don't allow multiple links to open
|
||||
// from one user gesture.
|
||||
// Avoid doing so for links opened in the current tab, which get
|
||||
// handled later, by gecko, as otherwise its popup blocker will stop
|
||||
// the link from opening.
|
||||
// We will do the same check (whereToOpenLink) again in the parent and
|
||||
// avoid handling the click for such links... but we still need the
|
||||
// click information in the parent because otherwise places link
|
||||
// tracking breaks. (bug 1742894 tracks improving this.)
|
||||
ownerDoc.consumeTransientUserGestureActivation();
|
||||
// We don't care about the return value because we already checked that
|
||||
// hasValidTransientUserGestureActivation was true earlier in this
|
||||
// function.
|
||||
}
|
||||
|
||||
json.href = href;
|
||||
if (node) {
|
||||
json.title = node.getAttribute("title");
|
||||
|
||||
@ -23,9 +23,85 @@ add_task(async function test_untrusted_click_opens_tab() {
|
||||
let eventObj = {};
|
||||
eventObj[AppConstants.platform == "macosx" ? "metaKey" : "ctrlKey"] = true;
|
||||
await BrowserTestUtils.synthesizeMouseAtCenter("#test", eventObj, browser);
|
||||
info("Waiting for new tab to open; if we timeout the test is broken.");
|
||||
info(
|
||||
"Waiting for new tab to open from frontend; if we timeout the test is broken."
|
||||
);
|
||||
let newTab = await newTabOpened;
|
||||
ok(newTab, "New tab should be opened.");
|
||||
BrowserTestUtils.removeTab(newTab);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Ensure that click handlers that redirect a modifier-less click into
|
||||
* an untrusted event without modifiers don't run afoul of the popup
|
||||
* blocker by having their transient user gesture bit consumed in the
|
||||
* child by the handling code for modified clicks.
|
||||
*/
|
||||
add_task(async function test_unused_click_doesnt_consume_activation() {
|
||||
// Enable the popup blocker.
|
||||
await SpecialPowers.pushPrefEnv({
|
||||
set: [
|
||||
["dom.disable_open_during_load", true],
|
||||
["dom.block_multiple_popups", true],
|
||||
],
|
||||
});
|
||||
await BrowserTestUtils.withNewTab(TEST_PATH + "click.html", async browser => {
|
||||
let newTabOpened = BrowserTestUtils.waitForNewTab(
|
||||
gBrowser,
|
||||
"https://example.com/",
|
||||
true
|
||||
);
|
||||
info("clicking button that generates link press.");
|
||||
await BrowserTestUtils.synthesizeMouseAtCenter(
|
||||
"#fire-untrusted",
|
||||
{},
|
||||
browser
|
||||
);
|
||||
info(
|
||||
"Waiting for new tab to open through gecko; if we timeout the test is broken."
|
||||
);
|
||||
let newTab = await newTabOpened;
|
||||
ok(newTab, "New tab should be opened.");
|
||||
BrowserTestUtils.removeTab(newTab);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Ensure that click handlers redirecting to elements without href don't
|
||||
* trigger user gesture consumption either.
|
||||
*/
|
||||
add_task(async function test_click_without_href_doesnt_consume_activation() {
|
||||
// Enable the popup blocker.
|
||||
await SpecialPowers.pushPrefEnv({
|
||||
set: [
|
||||
["dom.disable_open_during_load", true],
|
||||
["dom.block_multiple_popups", true],
|
||||
],
|
||||
});
|
||||
await BrowserTestUtils.withNewTab(TEST_PATH + "click.html", async browser => {
|
||||
let newTabOpened = BrowserTestUtils.waitForNewTab(gBrowser, "about:blank");
|
||||
info("clicking link that generates link click on target-less link.");
|
||||
await BrowserTestUtils.synthesizeMouseAtCenter(
|
||||
"#fire-targetless-link",
|
||||
{},
|
||||
browser
|
||||
);
|
||||
info(
|
||||
"Waiting for new tab to open after targetless link; if we timeout the test is broken."
|
||||
);
|
||||
let newTab = await newTabOpened;
|
||||
ok(newTab, "New tab should be opened.");
|
||||
BrowserTestUtils.removeTab(newTab);
|
||||
|
||||
newTabOpened = BrowserTestUtils.waitForNewTab(gBrowser, "about:blank");
|
||||
info("clicking link that generates button click.");
|
||||
await BrowserTestUtils.synthesizeMouseAtCenter("#fire-button", {}, browser);
|
||||
info(
|
||||
"Waiting for new tab to open after button redirect; if we timeout the test is broken."
|
||||
);
|
||||
newTab = await newTabOpened;
|
||||
ok(newTab, "New tab should be opened.");
|
||||
BrowserTestUtils.removeTab(newTab);
|
||||
});
|
||||
});
|
||||
|
||||
@ -6,7 +6,13 @@
|
||||
</head>
|
||||
<body>
|
||||
|
||||
<a id="test" href="https://example.com/">click me</a>
|
||||
<a id="test" href="https://example.com/">click me</a><br><br>
|
||||
|
||||
<a id="untrusted-only" href="https://example.com/" target="_blank">This is a link, works only with untrusted events</a><br>
|
||||
<input id="fire-untrusted" type=button value="Click me to trigger an untrusted event"><br><br>
|
||||
|
||||
<a id="fire-targetless-link" href="unused">Click me (dispatch untrusted event to link without href)</a><br>
|
||||
<a id="fire-button" href="unused">Click me (dispatch untrusted event to button)</a>
|
||||
|
||||
<script>
|
||||
|
||||
@ -33,6 +39,38 @@
|
||||
originalClick.stopPropagation();
|
||||
});
|
||||
|
||||
document.getElementById("untrusted-only").addEventListener("click", ev => {
|
||||
if (ev.isTrusted) {
|
||||
ev.preventDefault();
|
||||
}
|
||||
});
|
||||
|
||||
document.getElementById("fire-untrusted").addEventListener("click", () => {
|
||||
document.getElementById("untrusted-only").dispatchEvent(new MouseEvent("click"));
|
||||
});
|
||||
|
||||
function handleTargetless(e) {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
|
||||
const buttonOrLink = e.target.id.includes("button") ? "button" : "a";
|
||||
const newElement = document.createElement(buttonOrLink);
|
||||
document.body.appendChild(newElement);
|
||||
|
||||
const evt = document.createEvent("MouseEvent");
|
||||
evt.initMouseEvent("click", true, true, window, 0, e.screenX, e.screenY,
|
||||
e.clientX, e.clientY, e.ctrlKey, e.altKey, e.shiftKey,
|
||||
e.metaKey, e.button, e.relatedTarget);
|
||||
newElement.dispatchEvent(evt);
|
||||
newElement.remove();
|
||||
|
||||
setTimeout(() => {
|
||||
window.open();
|
||||
}, 0);
|
||||
}
|
||||
document.getElementById("fire-targetless-link").addEventListener("click", handleTargetless);
|
||||
document.getElementById("fire-button").addEventListener("click", handleTargetless);
|
||||
|
||||
</script>
|
||||
</body>
|
||||
</html>
|
||||
|
||||
@ -24,7 +24,6 @@ XPCOMUtils.defineLazyModuleGetters(this, {
|
||||
BrowserUsageTelemetry: "resource:///modules/BrowserUsageTelemetry.jsm",
|
||||
BrowserTelemetryUtils: "resource://gre/modules/BrowserTelemetryUtils.jsm",
|
||||
BrowserUIUtils: "resource:///modules/BrowserUIUtils.jsm",
|
||||
BrowserUtils: "resource://gre/modules/BrowserUtils.jsm",
|
||||
BrowserWindowTracker: "resource:///modules/BrowserWindowTracker.jsm",
|
||||
CFRPageActions: "resource://activity-stream/lib/CFRPageActions.jsm",
|
||||
Color: "resource://gre/modules/Color.jsm",
|
||||
|
||||
@ -14,6 +14,7 @@ var { XPCOMUtils } = ChromeUtils.import(
|
||||
|
||||
XPCOMUtils.defineLazyModuleGetters(this, {
|
||||
AboutNewTab: "resource:///modules/AboutNewTab.jsm",
|
||||
BrowserUtils: "resource://gre/modules/BrowserUtils.jsm",
|
||||
BrowserWindowTracker: "resource:///modules/BrowserWindowTracker.jsm",
|
||||
ContextualIdentityService:
|
||||
"resource://gre/modules/ContextualIdentityService.jsm",
|
||||
@ -161,93 +162,14 @@ function openUILink(
|
||||
openUILinkIn(url, where, params);
|
||||
}
|
||||
|
||||
// Utility function to check command events for potential middle-click events
|
||||
// from checkForMiddleClick and unwrap them.
|
||||
// This is here for historical reasons. bug 1742889 covers cleaning this up.
|
||||
function getRootEvent(aEvent) {
|
||||
// Part of the fix for Bug 1523813.
|
||||
// Middle-click events arrive here wrapped in different numbers (1-2) of
|
||||
// command events, depending on the button originally clicked.
|
||||
if (!aEvent) {
|
||||
return aEvent;
|
||||
}
|
||||
let tempEvent = aEvent;
|
||||
while (tempEvent.sourceEvent) {
|
||||
if (tempEvent.sourceEvent.button == 1) {
|
||||
aEvent = tempEvent.sourceEvent;
|
||||
break;
|
||||
}
|
||||
tempEvent = tempEvent.sourceEvent;
|
||||
}
|
||||
return aEvent;
|
||||
return BrowserUtils.getRootEvent(aEvent);
|
||||
}
|
||||
|
||||
/**
|
||||
* whereToOpenLink() looks at an event to decide where to open a link.
|
||||
*
|
||||
* The event may be a mouse event (click, double-click, middle-click) or keypress event (enter).
|
||||
*
|
||||
* On Windows, the modifiers are:
|
||||
* Ctrl new tab, selected
|
||||
* Shift new window
|
||||
* Ctrl+Shift new tab, in background
|
||||
* Alt save
|
||||
*
|
||||
* Middle-clicking is the same as Ctrl+clicking (it opens a new tab).
|
||||
*
|
||||
* Exceptions:
|
||||
* - Alt is ignored for menu items selected using the keyboard so you don't accidentally save stuff.
|
||||
* (Currently, the Alt isn't sent here at all for menu items, but that will change in bug 126189.)
|
||||
* - Alt is hard to use in context menus, because pressing Alt closes the menu.
|
||||
* - Alt can't be used on the bookmarks toolbar because Alt is used for "treat this as something draggable".
|
||||
* - The button is ignored for the middle-click-paste-URL feature, since it's always a middle-click.
|
||||
*
|
||||
* @param e {Event|Object} Event or JSON Object
|
||||
* @param ignoreButton {Boolean}
|
||||
* @param ignoreAlt {Boolean}
|
||||
* @returns {"current" | "tabshifted" | "tab" | "save" | "window"}
|
||||
*/
|
||||
// This is here for historical reasons. bug 1742889 covers cleaning this up.
|
||||
function whereToOpenLink(e, ignoreButton, ignoreAlt) {
|
||||
// This method must treat a null event like a left click without modifier keys (i.e.
|
||||
// e = { shiftKey:false, ctrlKey:false, metaKey:false, altKey:false, button:0 })
|
||||
// for compatibility purposes.
|
||||
if (!e) {
|
||||
return "current";
|
||||
}
|
||||
|
||||
e = getRootEvent(e);
|
||||
|
||||
var shift = e.shiftKey;
|
||||
var ctrl = e.ctrlKey;
|
||||
var meta = e.metaKey;
|
||||
var alt = e.altKey && !ignoreAlt;
|
||||
|
||||
// ignoreButton allows "middle-click paste" to use function without always opening in a new window.
|
||||
let middle = !ignoreButton && e.button == 1;
|
||||
let middleUsesTabs = Services.prefs.getBoolPref(
|
||||
"browser.tabs.opentabfor.middleclick",
|
||||
true
|
||||
);
|
||||
let middleUsesNewWindow = Services.prefs.getBoolPref(
|
||||
"middlemouse.openNewWindow",
|
||||
false
|
||||
);
|
||||
|
||||
// Don't do anything special with right-mouse clicks. They're probably clicks on context menu items.
|
||||
|
||||
var metaKey = AppConstants.platform == "macosx" ? meta : ctrl;
|
||||
if (metaKey || (middle && middleUsesTabs)) {
|
||||
return shift ? "tabshifted" : "tab";
|
||||
}
|
||||
|
||||
if (alt && Services.prefs.getBoolPref("browser.altClickSave", false)) {
|
||||
return "save";
|
||||
}
|
||||
|
||||
if (shift || (middle && !middleUsesTabs && middleUsesNewWindow)) {
|
||||
return "window";
|
||||
}
|
||||
|
||||
return "current";
|
||||
return BrowserUtils.whereToOpenLink(e, ignoreButton, ignoreAlt);
|
||||
}
|
||||
|
||||
/* openTrustedLinkIn will attempt to open the given URI using the SystemPrincipal
|
||||
|
||||
@ -7,6 +7,9 @@
|
||||
|
||||
var EXPORTED_SYMBOLS = ["BrowserUtils"];
|
||||
|
||||
const { AppConstants } = ChromeUtils.import(
|
||||
"resource://gre/modules/AppConstants.jsm"
|
||||
);
|
||||
const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
|
||||
const { XPCOMUtils } = ChromeUtils.import(
|
||||
"resource://gre/modules/XPCOMUtils.jsm"
|
||||
@ -205,6 +208,95 @@ var BrowserUtils = {
|
||||
node && node.ownerDocument.nodePrincipal,
|
||||
];
|
||||
},
|
||||
|
||||
/**
|
||||
* whereToOpenLink() looks at an event to decide where to open a link.
|
||||
*
|
||||
* The event may be a mouse event (click, double-click, middle-click) or keypress event (enter).
|
||||
*
|
||||
* On Windows, the modifiers are:
|
||||
* Ctrl new tab, selected
|
||||
* Shift new window
|
||||
* Ctrl+Shift new tab, in background
|
||||
* Alt save
|
||||
*
|
||||
* Middle-clicking is the same as Ctrl+clicking (it opens a new tab).
|
||||
*
|
||||
* Exceptions:
|
||||
* - Alt is ignored for menu items selected using the keyboard so you don't accidentally save stuff.
|
||||
* (Currently, the Alt isn't sent here at all for menu items, but that will change in bug 126189.)
|
||||
* - Alt is hard to use in context menus, because pressing Alt closes the menu.
|
||||
* - Alt can't be used on the bookmarks toolbar because Alt is used for "treat this as something draggable".
|
||||
* - The button is ignored for the middle-click-paste-URL feature, since it's always a middle-click.
|
||||
*
|
||||
* @param e {Event|Object} Event or JSON Object
|
||||
* @param ignoreButton {Boolean}
|
||||
* @param ignoreAlt {Boolean}
|
||||
* @returns {"current" | "tabshifted" | "tab" | "save" | "window"}
|
||||
*/
|
||||
whereToOpenLink(e, ignoreButton, ignoreAlt) {
|
||||
// This method must treat a null event like a left click without modifier keys (i.e.
|
||||
// e = { shiftKey:false, ctrlKey:false, metaKey:false, altKey:false, button:0 })
|
||||
// for compatibility purposes.
|
||||
if (!e) {
|
||||
return "current";
|
||||
}
|
||||
|
||||
e = this.getRootEvent(e);
|
||||
|
||||
var shift = e.shiftKey;
|
||||
var ctrl = e.ctrlKey;
|
||||
var meta = e.metaKey;
|
||||
var alt = e.altKey && !ignoreAlt;
|
||||
|
||||
// ignoreButton allows "middle-click paste" to use function without always opening in a new window.
|
||||
let middle = !ignoreButton && e.button == 1;
|
||||
let middleUsesTabs = Services.prefs.getBoolPref(
|
||||
"browser.tabs.opentabfor.middleclick",
|
||||
true
|
||||
);
|
||||
let middleUsesNewWindow = Services.prefs.getBoolPref(
|
||||
"middlemouse.openNewWindow",
|
||||
false
|
||||
);
|
||||
|
||||
// Don't do anything special with right-mouse clicks. They're probably clicks on context menu items.
|
||||
|
||||
var metaKey = AppConstants.platform == "macosx" ? meta : ctrl;
|
||||
if (metaKey || (middle && middleUsesTabs)) {
|
||||
return shift ? "tabshifted" : "tab";
|
||||
}
|
||||
|
||||
if (alt && Services.prefs.getBoolPref("browser.altClickSave", false)) {
|
||||
return "save";
|
||||
}
|
||||
|
||||
if (shift || (middle && !middleUsesTabs && middleUsesNewWindow)) {
|
||||
return "window";
|
||||
}
|
||||
|
||||
return "current";
|
||||
},
|
||||
|
||||
// Utility function to check command events for potential middle-click events
|
||||
// from checkForMiddleClick and unwrap them.
|
||||
getRootEvent(aEvent) {
|
||||
// Part of the fix for Bug 1523813.
|
||||
// Middle-click events arrive here wrapped in different numbers (1-2) of
|
||||
// command events, depending on the button originally clicked.
|
||||
if (!aEvent) {
|
||||
return aEvent;
|
||||
}
|
||||
let tempEvent = aEvent;
|
||||
while (tempEvent.sourceEvent) {
|
||||
if (tempEvent.sourceEvent.button == 1) {
|
||||
aEvent = tempEvent.sourceEvent;
|
||||
break;
|
||||
}
|
||||
tempEvent = tempEvent.sourceEvent;
|
||||
}
|
||||
return aEvent;
|
||||
},
|
||||
};
|
||||
|
||||
XPCOMUtils.defineLazyPreferenceGetter(
|
||||
|
||||
Loading…
Reference in New Issue
Block a user