corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1641171 - Have a better way to handle keyboard shortcut for user activation; r=masayuki,media-playback-reviewers,alwu

Browse Source 
We used to filter out all non-printable keys and printable keys combined with
other modifier keys to prevent web content from being activated when users used
certain shortcuts to interact with the browser. However, this was too strict,
as most key combinations are not used for browser shortcuts.

In this patch, we check whether a key handler is registered to execute command
for the key combination. If no handler is found, the key event is considered user
input for interacting with web content, and user activiation is generated.

Note that we cannot reuse the `mWantReplyFromContentProcess` event flag directly
because user activation is generated only on the keydown event, whereas key handlers
are ususally registered for the keypress event.

Differential Revision: https://phabricator.services.mozilla.com/D240006
This commit is contained in:
Edgar Chen
2025-03-12 17:03:48 +00:00
parent 59114cd149
commit 243a21b18c
10 changed files with 211 additions and 126 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
dom/base/test/useractivation/browser.toml Normal file
View File

@@ -0,0 +1,6 @@
[DEFAULT]
support-files = [
"../empty.html",
]
["browser_useractivation_key_events.js"]

106
dom/base/test/useractivation/browser_useractivation_key_events.js Normal file
View File

@@ -0,0 +1,106 @@
/* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/publicdomain/zero/1.0/ */
"use strict";
const BASE = getRootDirectory(gTestPath).replace(
"chrome://mochitests/content",
// eslint-disable-next-line @microsoft/sdl/no-insecure-url
"http://example.com"
);
const TEST_URL = BASE + "empty.html";
async function synthesizeKeyAndTest(aBrowser, aKey, aEvent, aIsActive) {
let promise = SpecialPowers.spawn(
aBrowser,
[aKey, aEvent, aIsActive],
async (key, event, isActive) => {
return new Promise(aResolve => {
content.document.clearUserGestureActivation();
content.document.addEventListener(
"keydown",
function (e) {
e.preventDefault();
is(
content.document.hasBeenUserGestureActivated,
isActive,
`check has-been-user-activated for ${key} with ${JSON.stringify(event)}`
);
is(
content.document.hasValidTransientUserGestureActivation,
isActive,
`check has-valid-transient-user-activation for ${key} with ${JSON.stringify(event)}`
);
aResolve();
},
{ once: true }
);
});
}
);
// Ensure the event listener has registered on the remote.
await SpecialPowers.spawn(aBrowser, [], () => {
return new Promise(resolve => {
SpecialPowers.executeSoon(resolve);
});
});
EventUtils.synthesizeKey(aKey, aEvent);
return promise;
}
let browser;
add_setup(async function setup() {
let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL);
browser = tab.linkedBrowser;
registerCleanupFunction(async () => {
BrowserTestUtils.removeTab(tab);
});
});
add_task(async function TestPrintableKey() {
let tests = ["a", "b", "c", "A", "B", "1", "2", "3"];
for (let key of tests) {
await synthesizeKeyAndTest(browser, key, {}, true);
}
});
add_task(async function TestNonPrintableKey() {
let tests = [
["KEY_Backspace", false],
["KEY_Control", false],
["KEY_Shift", false],
["KEY_Escape", false],
// Treat as user input
["KEY_Tab", true],
["KEY_Enter", true],
[" ", true],
];
for (let [key, expectedResult] of tests) {
await synthesizeKeyAndTest(browser, key, {}, expectedResult);
}
});
add_task(async function TestModifier() {
let tests = [
["a", { accelKey: true }, false],
["z", { accelKey: true }, false],
["a", { metaKey: true }, !navigator.platform.includes("Mac")],
// Treat as user input
["a", { altGraphKey: true }, true],
["a", { fnKey: true }, true],
["a", { altKey: true }, true],
["a", { shiftKey: true }, true],
["c", { altKey: true }, true],
["c", { accelKey: true }, true],
["v", { altKey: true }, true],
["v", { accelKey: true }, true],
["x", { altKey: true }, true],
["x", { accelKey: true }, true],
];
for (let [key, event, expectedResult] of tests) {
await synthesizeKeyAndTest(browser, key, event, expectedResult);
}
});

2
dom/base/test/useractivation/mochitest.toml
View File

@@ -30,8 +30,6 @@ support-files = ["file_self_close.html"]
["test_useractivation_has_been_activated.html"]
["test_useractivation_key_events.html"]
["test_useractivation_open_new_window.html"]
support-files = ["file_self_close.html"]

4
dom/base/test/useractivation/moz.build
View File

@@ -11,3 +11,7 @@ MOCHITEST_MANIFESTS += [
MOCHITEST_CHROME_MANIFESTS += [
"chrome.toml",
]
BROWSER_CHROME_MANIFESTS += [
"browser.toml",
]

87
dom/base/test/useractivation/test_useractivation_key_events.html
View File

@@ -1,87 +0,0 @@
<!DOCTYPE HTML>
<html>
<head>
<title>User activation test: key events</title>
<script src="/tests/SimpleTest/SimpleTest.js"></script>
<script src="/tests/SimpleTest/EventUtils.js"></script>
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
</head>
<body>
<script>
function synthesizeKeyAndWait(aKey, aEvent) {
let promise = new Promise(aResolve => {
document.addEventListener("keydown", function(e) {
e.preventDefault();
aResolve();
}, { once: true });
});
synthesizeKey(aKey, aEvent, window);
return promise;
}
add_task(async function TestPrintableKey() {
let tests = [ 'a', 'b', 'c', 'A', 'B', '1', '2', '3' ];
for (let key of tests) {
SpecialPowers.wrap(document).clearUserGestureActivation();
await synthesizeKeyAndWait(key, {});
ok(SpecialPowers.wrap(document).hasBeenUserGestureActivated,
`check has-been-user-activated for ${key}`);
ok(SpecialPowers.wrap(document).hasValidTransientUserGestureActivation,
`check has-valid-transient-user-activation for ${key}`);
}
});
add_task(async function TestNonPrintableKey() {
let tests = [ [ "KEY_Alt", false],
[ "KEY_Backspace", false],
[ "KEY_Escape" , false ],
[ "KEY_Tab" , false ],
// Treat as user input
[ "KEY_Enter", true],
[ " ", true] ];
for (let [key, expectedResult] of tests) {
SpecialPowers.wrap(document).clearUserGestureActivation();
await synthesizeKeyAndWait(key, {});
is(SpecialPowers.wrap(document).hasBeenUserGestureActivated, expectedResult,
`check has-been-user-activated for "${key}"`);
is(SpecialPowers.wrap(document).hasValidTransientUserGestureActivation, expectedResult,
`check has-valid-transient-user-activation for "${key}"`);
}
});
add_task(async function TestModifier() {
let tests = [ [ 'a', { altKey: true }, false],
[ 'a', { ctrlKey: true }, false],
[ 'a', { metaKey: true }, false],
[ 'c', { altKey: true }, false ],
[ 'v', { altKey: true }, false ],
[ 'x', { altKey: true }, false ],
// Treat as user input
[ 'a', { altGraphKey: true }, true ],
[ 'a', { fnKey: true }, true ],
[ 'a', { shiftKey: true }, true ],
[ 'c', { accelKey: true }, true ],
[ 'v', { accelKey: true }, true ],
[ 'x', { accelKey: true }, true ] ];
for (let [key, event, expectedResult] of tests) {
SpecialPowers.wrap(document).clearUserGestureActivation();
await synthesizeKeyAndWait(key, event);
is(SpecialPowers.wrap(document).hasBeenUserGestureActivated, expectedResult,
`check has-been-user-activated for ${key} with ${JSON.stringify(event)}`);
is(SpecialPowers.wrap(document).hasValidTransientUserGestureActivation, expectedResult,
`check has-valid-transient-user-activation for ${key} with ${JSON.stringify(event)}`);
}
});
add_task(async function endTests() {
// Reset the activation flag in order not to interfere following test in the
// verify mode which would run the test using same document couple times.
SpecialPowers.wrap(document).clearUserGestureActivation();
});
</script>
</body>

16
dom/events/GlobalKeyListener.cpp
View File

@@ -223,7 +223,9 @@ void GlobalKeyListener::HandleEventOnCaptureInSystemEventGroup(
return;
}
if (!HasHandlerForEvent(aEvent).mMeaningfulHandlerFound) {
WalkHandlersResult result = HasHandlerForEvent(aEvent);
widgetEvent->mFlags.mIsShortcutKey |= result.mRelevantHandlerFound;
if (!result.mMeaningfulHandlerFound) {
return;
}
@@ -261,6 +263,7 @@ GlobalKeyListener::WalkHandlersResult GlobalKeyListener::WalkHandlersInternal(
}
bool foundDisabledHandler = false;
bool foundRelevantHandler = false;
for (const ShortcutKeyCandidate& key : shortcutKeys) {
const bool skipIfEarlierHandlerDisabled =
key.mSkipIfEarlierHandlerDisabled ==
@@ -276,6 +279,7 @@ GlobalKeyListener::WalkHandlersResult GlobalKeyListener::WalkHandlersInternal(
if (result.mMeaningfulHandlerFound) {
return result;
}
foundRelevantHandler |= result.mRelevantHandlerFound;
// Note that if the candidate should not match if an earlier handler is
// disabled, the char code of the candidate is a char which may be
// introduced with different shift state. In this case, we do NOT find a
@@ -286,7 +290,9 @@ GlobalKeyListener::WalkHandlersResult GlobalKeyListener::WalkHandlersInternal(
foundDisabledHandler = result.mDisabledHandlerFound;
}
}
return {};
WalkHandlersResult result;
result.mRelevantHandlerFound = foundRelevantHandler;
return result;
}
GlobalKeyListener::WalkHandlersResult GlobalKeyListener::WalkHandlersAndExecute(
@@ -303,6 +309,7 @@ GlobalKeyListener::WalkHandlersResult GlobalKeyListener::WalkHandlersAndExecute(
// Try all of the handlers until we find one that matches the event.
bool foundDisabledHandler = false;
bool foundRelevantHandler = false;
for (KeyEventHandler* handler = mHandler; handler;
handler = handler->GetNextHandler()) {
bool stopped = aKeyEvent->IsDispatchStopped();
@@ -352,6 +359,7 @@ GlobalKeyListener::WalkHandlersResult GlobalKeyListener::WalkHandlersAndExecute(
result.mMeaningfulHandlerFound = true;
result.mReservedHandlerForChromeFound =
IsReservedKey(widgetKeyboardEvent, handler);
result.mRelevantHandlerFound = true;
return result;
}
@@ -364,8 +372,10 @@ GlobalKeyListener::WalkHandlersResult GlobalKeyListener::WalkHandlersAndExecute(
WalkHandlersResult result;
result.mMeaningfulHandlerFound = true;
result.mReservedHandlerForChromeFound = true;
result.mRelevantHandlerFound = true;
return result;
}
foundRelevantHandler = true;
}
// Otherwise, we've not found a handler for the event yet.
continue;
@@ -382,6 +392,7 @@ GlobalKeyListener::WalkHandlersResult GlobalKeyListener::WalkHandlersAndExecute(
result.mReservedHandlerForChromeFound =
IsReservedKey(widgetKeyboardEvent, handler);
result.mDisabledHandlerFound = (rv == NS_SUCCESS_DOM_NO_OPERATION);
result.mRelevantHandlerFound = true;
return result;
}
}
@@ -401,6 +412,7 @@ GlobalKeyListener::WalkHandlersResult GlobalKeyListener::WalkHandlersAndExecute(
WalkHandlersResult result;
result.mDisabledHandlerFound = foundDisabledHandler;
result.mRelevantHandlerFound = foundRelevantHandler;
return result;
}

3
dom/events/GlobalKeyListener.h
View File

@@ -69,6 +69,9 @@ class GlobalKeyListener : public nsIDOMEventListener {
bool mReservedHandlerForChromeFound = false;
// Set to true if found handler is disabled.
bool mDisabledHandlerFound = false;
// Set to true if a command is found but may correspond to a different type
// of keyboard event.
bool mRelevantHandlerFound = false;
};
// walk the handlers, looking for one to handle the event

65
dom/media/autoplay/test/mochitest/file_autoplay_policy_key_blacklist.html
View File

@@ -31,15 +31,11 @@
// Keys that are expected to be not considered interaction with the page, and
// so not gesture activate the document.
let blacklistKeyPresses = [
"Tab",
"CapsLock",
"NumLock",
"ScrollLock",
"FnLock",
"Meta",
"Hyper",
"Super",
"ContextMenu",
"ArrowUp",
"ArrowDown",
"ArrowLeft",
@@ -48,7 +44,6 @@
"PageDown",
"Home",
"End",
"Backspace",
"Fn",
"Alt",
"AltGraph",
@@ -57,14 +52,58 @@
"Escape",
];
// XXX not sure why Android behave different on handling Backspace key.
if (!navigator.appVersion.includes("Android")) {
blacklistKeyPresses.push("Backspace");
}
let modifiedKeys = [
{ key: "V", modifiers: { altKey: true, shiftKey: true } },
{ key: "a", modifiers: { altKey: true } },
{ key: "a", modifiers: { ctrlKey: true } },
{ key: "KEY_ArrowRight", modifiers: { metaKey: true } },
{ key: "KEY_ArrowRight", modifiers: { altKey: true } },
{ key: "a", modifiers: { accelKey: true } },
{ key: "z", modifiers: { accelKey: true } },
];
// Browser shortcut on non-Linux platform.
if (navigator.platform.indexOf("Linux") !== 0) {
modifiedKeys.push(
{ key: "KEY_ArrowRight", modifiers: { metaKey: true } },
{ key: "KEY_ArrowRight", modifiers: { altKey: true } });
}
let script = SpecialPowers.loadChromeScript(function() {
/* eslint-env mozilla/chrome-script */
var EventUtils = {};
EventUtils.window = {};
EventUtils._EU_Ci = Ci;
EventUtils._EU_Cc = Cc;
Services.scriptloader
.loadSubScript("chrome://mochikit/content/tests/SimpleTest/EventUtils.js",
EventUtils);
addMessageListener("synthesizeKey", function(data) {
let browserWin = Services.wm.getMostRecentWindow("navigator:browser");
if (!browserWin) {
browserWin = Services.wm.getMostRecentWindow("navigator:geckoview");
}
EventUtils.synthesizeKey(data.key, data.event, browserWin);
});
});
async function synthesizeKeyAndWait(key, event = {}) {
let promise = new Promise(aResolve => {
document.addEventListener(
"keydown",
function (e) {
e.preventDefault();
info("Received keydown event: " + e.key);
aResolve();
},
{ once: true }
);
});
script.sendAsyncMessage("synthesizeKey", { key, event });
await promise;
}
async function sendInput(element, name, input) {
synthesizeMouseAtCenter(input, {});
let played = await element.play().then(() => true, () => false);
@@ -107,7 +146,7 @@
for (let key of blacklistKeyPresses) {
document.body.focus();
synthesizeKey("KEY_" + key);
await synthesizeKeyAndWait("KEY_" + key);
played = await element.play().then(() => true, () => false);
is(played, false, "Key " + key + " should not activate document and should not unblock play");
}
@@ -116,14 +155,14 @@
let keyNames = (m) => Object.keys(m).join("+");
for (let x of modifiedKeys) {
document.body.focus();
synthesizeKey(x.key, x.modifiers);
await synthesizeKeyAndWait(x.key, x.modifiers);
played = await element.play().then(() => true, () => false);
is(played, false, "Key (" + x.key + "+" + keyNames(x.modifiers) + ") should not activate document and should not unblock play");
}
// Try pressing a key not in the blacklist, then playing.
// Document should be activated, and media should play.
synthesizeKey(" ");
await synthesizeKeyAndWait(" ");
played = await element.play().then(() => true, () => false);
is(played, true, "Space key should activate document and should unblock play");

3
widget/BasicEvents.h
View File

@@ -162,6 +162,9 @@ struct BaseEventFlags {
// Certain mouse events can be marked as positionless to return 0 from
// coordinate related getters.
bool mIsPositionless : 1;
// Indicates if a key handler is registered to execute a command for the key
// combination.
bool mIsShortcutKey : 1;
// Flags managing state of propagation between processes.
// Note the the following flags shouldn't be referred directly. Use utility

45
widget/TextEvents.h
View File

@@ -310,28 +310,29 @@ class WidgetKeyboardEvent final : public WidgetInputEvent {
}
bool CanUserGestureActivateTarget() const {
// Printable keys, 'carriage return' and 'space' are supported user gestures
// for activating the document. However, if supported key is being pressed
// combining with other operation keys, such like alt, control ..etc., we
// won't activate the target for them because at that time user might
// interact with browser or window manager which doesn't necessarily
// demonstrate user's intent to play media.
const bool isCombiningWithOperationKeys = (IsControl() && !IsAltGraph()) ||
(IsAlt() && !IsAltGraph()) ||
IsMeta();
const bool isEnterOrSpaceKey =
mKeyNameIndex == KEY_NAME_INDEX_Enter || mKeyCode == NS_VK_SPACE;
return (PseudoCharCode() || isEnterOrSpaceKey) &&
(!isCombiningWithOperationKeys ||
// ctrl-c/ctrl-x/ctrl-v is quite common shortcut for clipboard
// operation.
// XXXedgar, we have to find a better way to handle browser keyboard
// shortcut for user activation, instead of just ignoring all
// combinations, see bug 1641171.
((mKeyCode == dom::KeyboardEvent_Binding::DOM_VK_C ||
mKeyCode == dom::KeyboardEvent_Binding::DOM_VK_V ||
mKeyCode == dom::KeyboardEvent_Binding::DOM_VK_X) &&
IsAccel()));
if (IsModifierKeyEvent()) {
return false;
}
if (mFlags.mIsShortcutKey) {
// Space is quite common shortcut for playing media.
return mKeyCode == NS_VK_SPACE ||
// ctrl-c/ctrl-x/ctrl-v is quite common shortcut for clipboard
// operation.
// XXXedgar, we probably could improve this by referring to
// EditCommandsConstRef() if we're sure the event target on Linux
// and macOS is active with any edit commands.
((mKeyCode == dom::KeyboardEvent_Binding::DOM_VK_C ||
mKeyCode == dom::KeyboardEvent_Binding::DOM_VK_V ||
mKeyCode == dom::KeyboardEvent_Binding::DOM_VK_X) &&
IsAccel());
}
// ESC key is ususally used to exit some state, it should not be considered
// as a user activation key to avoid page requests to enter again the same
// state to trap the user.
// https://html.spec.whatwg.org/multipage/interaction.html#activation-triggering-input-event
return mKeyNameIndex != KEY_NAME_INDEX_Escape;
}
// Returns true if this event is likely an user activation for a link or