Bug 1879765 - part 2: Make BrowserChild store the last code value of consumed eKeyDown event r=smaug

The builtin legacy IME of Windows to type a Unicode with typing a code point
causes consumed `eKeyDown` events while typing the code point, i.e., without
`eKeyPress` (FYI: The consumed state is not exposed to the web, it's used only
in chrome UI for the compatibility with Chrome).  Then, `BrowserChild` store
whether the last `eKeyDown` was consumed or not to prevent the following
`eKeyPress`.  Finally, a `eKeyPress` event is fired to input the Unicode
character after `eKeyUp` for `Alt`.  The stored value is set to new value only
when another `eKeyDown` event is sent from the parent process.  Therefore,
the last digit inputting `eKeyDown` causes `BrowserChild` thinking the last
`eKeyDown` is consumed so that the last `eKeyPress` is not dispatched.

This patch makes `BrowserChild` to store the `code` value of the last consumed
`eKeyDown` and check the `code` value to consider whether coming `eKeyPress`
should be or not be dispatched to `PresShell` and the DOM.

Differential Revision: https://phabricator.services.mozilla.com/D207957
This commit is contained in:
Masayuki Nakano 2024-05-15 05:42:33 +00:00
parent 4580f0c8df
commit 06ef42c72c
7 changed files with 380 additions and 5 deletions

View File

@ -1250,6 +1250,71 @@ nsresult TextInputProcessor::KeyupInternal(
return NS_OK;
}
NS_IMETHODIMP
TextInputProcessor::InsertTextWithKeyPress(const nsAString& aString,
Event* aDOMKeyEvent,
uint32_t aKeyFlags,
uint8_t aOptionalArgc,
bool* aDoDefault) {
MOZ_RELEASE_ASSERT(aDoDefault, "aDoDefault must not be nullptr");
MOZ_RELEASE_ASSERT(nsContentUtils::IsCallerChrome());
nsresult rv = IsValidStateForComposition();
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
if (aOptionalArgc < 1) {
aDOMKeyEvent = nullptr;
}
if (aOptionalArgc < 2) {
aKeyFlags = 0;
}
*aDoDefault = !(aKeyFlags & KEY_DEFAULT_PREVENTED);
WidgetKeyboardEvent* const originalKeyEvent =
aDOMKeyEvent ? aDOMKeyEvent->WidgetEventPtr()->AsKeyboardEvent()
: nullptr;
if (NS_WARN_IF(aDOMKeyEvent && !originalKeyEvent)) {
return NS_ERROR_INVALID_ARG;
}
WidgetKeyboardEvent keyEvent(true, eKeyPress, nullptr);
if (originalKeyEvent) {
keyEvent = WidgetKeyboardEvent(*originalKeyEvent);
keyEvent.mFlags.mIsTrusted = true;
keyEvent.mMessage = eKeyPress;
}
keyEvent.mKeyNameIndex = KEY_NAME_INDEX_USE_STRING;
keyEvent.mKeyValue = aString;
rv = PrepareKeyboardEventToDispatch(keyEvent, aKeyFlags);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
// Do not dispatch modifier key events even if the source event is a modifier
// key event because modifier state should be changed before this.
// TODO: In some test scenarios, we may need a new flag to use the given
// modifier state as-is.
keyEvent.mModifiers = GetActiveModifiers();
// See KeyDownInternal() for the detail of this.
if (XRE_IsContentProcess()) {
nsresult rv = InitEditCommands(keyEvent);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
}
nsEventStatus status =
*aDoDefault ? nsEventStatus_eIgnore : nsEventStatus_eConsumeNoDefault;
RefPtr<TextEventDispatcher> dispatcher(mDispatcher);
if (dispatcher->MaybeDispatchKeypressEvents(keyEvent, status)) {
*aDoDefault = (status != nsEventStatus_eConsumeNoDefault);
}
return NS_OK;
}
NS_IMETHODIMP
TextInputProcessor::GetModifierState(const nsAString& aModifierKeyName,
bool* aActive) {

View File

@ -3648,6 +3648,125 @@ async function runKeyTests()
window.removeEventListener("keyup", handler);
}
function runInsertTextWithKeyPressTests() {
const description = "runInsertTextWithKeyPressTests(): ";
var TIP = createTIP();
ok(TIP.beginInputTransactionForTests(window),
description + "TIP.beginInputTransactionForTests() should succeed");
input.value = "";
input.focus();
let events = [];
function handler(aEvent) {
events.push({
type: aEvent.type,
key: aEvent.key,
code: aEvent.code,
shiftKey: aEvent.shiftKey,
altKey: aEvent.altKey,
ctrlKey: aEvent.ctrlKey,
metaKey: aEvent.metaKey,
});
}
function stringifyEvents(aEvents) {
if (!aEvents.length) {
return "[]";
}
function stringifyEvent(aEvent) {
return `{ type: "${aEvent.type}", key: "${aEvent.key}", code: ${aEvent.code}, shiftKey: ${
aEvent.shiftKey
}, altKey: ${aEvent.altKey}, ctrlKey: ${aEvent.ctrlKey}, metaKey: ${aEvent.metaKey}}`;
}
let result = "";
for (const event of aEvents) {
if (result == "") {
result = "[ ";
} else {
result += ", ";
}
result += stringifyEvent(event);
}
return result + " ]";
}
window.addEventListener("keydown", handler);
window.addEventListener("keypress", handler);
window.addEventListener("keyup", handler);
events = [];
input.value = "";
TIP.insertTextWithKeyPress("X");
is(
input.value,
"X",
`${description}insertTextWithKeyPress without optional args should cause inserting the string`
);
is(
stringifyEvents(events),
stringifyEvents([
{ type: "keypress", key: "X", code: "", shiftKey: false, altKey: false, ctrlKey: false, metaKey: false },
]),
`${description}insertTextWithPress without optional args should cause only a "keypress" event`
);
events = [];
input.value = "";
TIP.insertTextWithKeyPress("Y", new KeyboardEvent("keydown", {key: "Alt", code: "AltLeft"}));
is(
input.value,
"Y",
`${description}insertTextWithKeyPress with Alt keydown event should cause inserting the string`
);
// The key value should be the inserted string and the code value should be same as the source event's.
is(
stringifyEvents(events),
stringifyEvents([
{ type: "keypress", key: "Y", code: "AltLeft", shiftKey: false, altKey: false, ctrlKey: false, metaKey: false },
]),
`${description}insertTextWithPress with Alt keydown event should cause only a "keypress" event whose code is "AltLeft"`
);
events = [];
input.value = "";
TIP.insertTextWithKeyPress("Z", new KeyboardEvent("keydown", {key: "Alt", code: "AltLeft", altKey: true}));
is(
input.value,
"Z",
`${description}insertTextWithKeyPress with Alt keydown whose altKey is true should cause inserting the string`
);
// TIP should use its internal modifier state instead of specified modifier state.
is(
stringifyEvents(events),
stringifyEvents([
{ type: "keypress", key: "Z", code: "AltLeft", shiftKey: false, altKey: false, ctrlKey: false, metaKey: false },
]),
`${description}insertTextWithPress with Alt keydown whose altKey is true should cause only a "keypress" event whose altKey is false"`
);
TIP.keydown(new KeyboardEvent("keydown", { key: "Alt" }));
events = [];
input.value = "";
TIP.insertTextWithKeyPress("X", new KeyboardEvent("keydown", {key: "Shift", code: "ShiftLeft", shiftKey: true}));
is(
input.value,
"",
`${description}insertTextWithKeyPress after Alt keydown should not cause inserting the string`
);
is(
stringifyEvents(events),
stringifyEvents([
{ type: "keypress", key: "X", code: "ShiftLeft", shiftKey: false, altKey: true, ctrlKey: false, metaKey: false },
]),
`${description}insertTextWithPress after Alt keydown should cause only a "keypress" event whose altKey is true"`
);
TIP.keyup(new KeyboardEvent("keyup ", { key: "Alt" }));
window.removeEventListener("keydown", handler);
window.removeEventListener("keypress", handler);
window.removeEventListener("keyup", handler);
}
function runErrorTests()
{
var description = "runErrorTests(): ";
@ -4854,6 +4973,7 @@ async function runTests()
runCompositionWithKeyEventTests();
runConsumingKeydownBeforeCompositionTests();
await runKeyTests();
runInsertTextWithKeyPressTests();
runErrorTests();
runCommitCompositionTests();
await runCallbackTests(false);

View File

@ -503,6 +503,8 @@ skip-if = ["os == 'android'"] # timeout
["test_unbound_before_in_active_chain.html"]
["test_unicode_input_on_windows_with_emulation.html"]
["test_use_conflated_keypress_event_model_on_newer_Office_Online_Server.html"]
["test_use_split_keypress_event_model_on_old_Confluence.html"]

View File

@ -0,0 +1,131 @@
<!doctype html>
<head>
<meta charset="utf-8">
<title>Test event sequence of Unicode character input of Windows builtin IME</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">
<script>
"use strict";
SimpleTest.waitForExplicitFinish();
SimpleTest.waitForFocus(async () => {
await SpecialPowers.pushPrefEnv({
set: [
["test.events.async.enabled", true],
],
});
const input = document.querySelector("input");
input.focus();
let events = [];
function handler(aEvent) {
events.push({
type: aEvent.type,
key: aEvent.key,
code: aEvent.code,
altKey: aEvent.altKey,
defaultPrevented: aEvent.defaultPrevented,
});
}
function stringifyEvents(aEvents) {
if (!aEvents.length) {
return "[]";
}
function stringifyEvent(aEvent) {
return `{ type: "${aEvent.type}", key: "${aEvent.key}", code: ${aEvent.code}, defaultPrevented: ${
aEvent.defaultPrevented
}`;
}
let result = "";
for (const event of aEvents) {
if (result == "") {
result = "[ ";
} else {
result += ", ";
}
result += stringifyEvent(event);
}
return result + " ]";
}
input.addEventListener("keydown", handler);
input.addEventListener("keypress", handler);
input.addEventListener("keyup", handler);
const waitForInput = new Promise(resolve => {
input.addEventListener("input", resolve, {once: true});
});
/**
* On Windows, users can enable a legacy Unicode input IME with adding a
* registry key, `EnableHexNumpad` whose type is `REG_SZ` and value is "1"
* under `HKEY_CURRENT_USER\Control Panel\Input Method`. Then, user can
* type a Unicode character with typing the code point in hex while holding
* Alt key and typing "+" in the numpad. Finally, when the Alt key is
* released, a Unicode character should be inserted into focused editable
* element. In this case, NativeKey class dispatches the typing "+" and
* hex values only with `keydown` and `keyup` events, then, dispatch only
* a "keypress" for the Unicode character.
*
* This test checks whether the events are dispatched as expected in a
* content process when it comes from the parent process.
*/
const nsITextInputProcessor = SpecialPowers.Ci.nsITextInputProcessor;
const TIP = SpecialPowers.Cc["@mozilla.org/text-input-processor;1"].createInstance(nsITextInputProcessor);
ok(TIP.beginInputTransactionForTests(window), "beginInputTransactionForTests should've succeeded");
TIP.keydown(new KeyboardEvent("keydown", { key: "Alt", code: "AltLeft" }));
TIP.keydown(new KeyboardEvent("keydown", { key: "+", code : "NumpadAdd" }), nsITextInputProcessor.KEY_DEFAULT_PREVENTED);
TIP.keyup(new KeyboardEvent("keyup", { key: "+", code : "NumpadAdd" }));
TIP.keydown(new KeyboardEvent("keydown", { key: "2", code : "Numpad2" }), nsITextInputProcessor.KEY_DEFAULT_PREVENTED);
TIP.keyup(new KeyboardEvent("keyup", { key: "2", code : "Numpad2" }));
TIP.keydown(new KeyboardEvent("keydown", { key: "7", code : "Numpad7" }), nsITextInputProcessor.KEY_DEFAULT_PREVENTED);
TIP.keyup(new KeyboardEvent("keyup", { key: "7", code : "Numpad7" }));
TIP.keydown(new KeyboardEvent("keydown", { key: "4", code : "Numpad4" }), nsITextInputProcessor.KEY_DEFAULT_PREVENTED);
TIP.keyup(new KeyboardEvent("keyup", { key: "4", code : "Numpad4" }));
TIP.keydown(new KeyboardEvent("keydown", { key: "e", code : "KeyE" }), nsITextInputProcessor.KEY_DEFAULT_PREVENTED);
TIP.keyup(new KeyboardEvent("keyup", { key: "e", code : "KeyE" }));
TIP.keyup(new KeyboardEvent("keyup", { key: "Alt", code: "AltLeft" }));
TIP.insertTextWithKeyPress("\u274e", new KeyboardEvent("keyup", { key: "Alt", code: "AltLeft" }));
info("Waiting for input event...");
await waitForInput;
is(
input.value,
"\u274e",
"Only the unicode character should be inserted"
);
is(
stringifyEvents(events),
stringifyEvents([
{ type: "keydown", key: "Alt", code: "AltLeft", altKey: true, defaultPrevented: false },
{ type: "keydown", key: "+", code: "NumpadAdd", altKey: true, defaultPrevented: false },
{ type: "keyup", key: "+", code: "NumpadAdd", altKey: true, defaultPrevented: false },
{ type: "keydown", key: "2", code: "Numpad2", altKey: true, defaultPrevented: false },
{ type: "keyup", key: "2", code: "Numpad2", altKey: true, defaultPrevented: false },
{ type: "keydown", key: "7", code: "Numpad7", altKey: true, defaultPrevented: false },
{ type: "keyup", key: "7", code: "Numpad7", altKey: true, defaultPrevented: false },
{ type: "keydown", key: "4", code: "Numpad4", altKey: true, defaultPrevented: false },
{ type: "keyup", key: "4", code: "Numpad4", altKey: true, defaultPrevented: false },
{ type: "keydown", key: "e", code: "KeyE", altKey: true, defaultPrevented: false },
{ type: "keyup", key: "e", code: "KeyE", altKey: true, defaultPrevented: false },
{ type: "keyup", key: "Alt", code: "AltLeft", altKey: false, defaultPrevented: false },
{ type: "keypress", key: "\u274e", code: "AltLeft", altKey: false, defaultPrevented: false },
]),
"Typing the code point should not cause keypress events but defaultPrevented should be false, " +
"and finally, the Unicode character should be inserted with a keypress event"
);
input.removeEventListener("keydown", handler);
input.removeEventListener("keypress", handler);
input.removeEventListener("keyup", handler);
SimpleTest.finish();
});
</script>
</head>
<body>
<input>
</body>
</html>

View File

@ -584,6 +584,25 @@ interface nsITextInputProcessor : nsISupports
boolean keyup(in Event aKeyboardEvent,
[optional] in unsigned long aKeyFlags);
/**
* insertTextWithKeyPress() is designed for automated tests. This dispatches
* isolated eKeyPress events to insert aString if selection is in an editable
* element.
*
* @aString The string which will be inserted.
* @aKeyboardEvent [optional] This is nullable. If specified as
* non-nullptr, the insertion is emulated as caused by the
* key event.
* @param aKeyFlags Special flags. The values can be some of KEY_*
* constants.
* @return false if one of the dispatched eKeyPress event is
* consumed.
*/
[can_run_script, optional_argc]
boolean insertTextWithKeyPress(in AString aString,
[optional] in Event aKeyboardEvent,
[optional] in unsigned long aKeyFlags);
/**
* getModifierState() returns modifier state managed by this instance.
*

View File

@ -278,7 +278,6 @@ BrowserChild::BrowserChild(ContentChild* aManager, const TabId& aTabId,
mUniqueId(aTabId),
mDidFakeShow(false),
mTriedBrowserInit(false),
mIgnoreKeyPressEvent(false),
mHasValidInnerSize(false),
mDestroyed(false),
mIsTopLevel(aIsTopLevel),
@ -1983,9 +1982,14 @@ mozilla::ipc::IPCResult BrowserChild::RecvRealKeyEvent(
aEvent.AreAllEditCommandsInitialized());
// If content code called preventDefault() on a keydown event, then we don't
// want to process any following keypress events.
// want to process any following keypress events which is caused by the
// preceding keydown (i.e., default action of the preceding keydown).
// In other words, if the keypress is not a default action of the preceding
// keydown, we should not stop dispatching keypress event even if the
// immediate preceding keydown was consumed.
const bool isPrecedingKeyDownEventConsumed =
aEvent.mMessage == eKeyPress && mIgnoreKeyPressEvent;
aEvent.mMessage == eKeyPress && mPreviousConsumedKeyDownCode.isSome() &&
mPreviousConsumedKeyDownCode.value() == aEvent.mCodeNameIndex;
WidgetKeyboardEvent localEvent(aEvent);
localEvent.mWidget = mPuppetWidget;
@ -1999,7 +2003,41 @@ mozilla::ipc::IPCResult BrowserChild::RecvRealKeyEvent(
UpdateRepeatedKeyEventEndTime(localEvent);
if (aEvent.mMessage == eKeyDown) {
mIgnoreKeyPressEvent = status == nsEventStatus_eConsumeNoDefault;
// If eKeyDown is consumed, we should stop dispatching the following
// eKeyPress events since the events are default action of eKeyDown.
// FIXME: We should synthesize eKeyPress in this process (bug 1181501).
if (status == nsEventStatus_eConsumeNoDefault) {
MOZ_ASSERT_IF(!aEvent.mFlags.mIsSynthesizedForTests,
aEvent.mCodeNameIndex != CODE_NAME_INDEX_USE_STRING);
// If mPreviousConsumedKeyDownCode is not Nothing, 2 or more keys may be
// pressed at same time and their eKeyDown are consumed. However, we
// forget the previous eKeyDown event result here and that might cause
// dispatching eKeyPress events caused by the previous eKeyDown in
// theory. However, this should not occur because eKeyPress should be
// fired before another eKeyDown, although it's depend on how the native
// keyboard event handler is implemented.
mPreviousConsumedKeyDownCode = Some(aEvent.mCodeNameIndex);
}
// If eKeyDown is not consumed but we know preceding eKeyDown is consumed,
// we need to forget it since we should not stop dispatching following
// eKeyPress events which are default action of current eKeyDown.
else if (mPreviousConsumedKeyDownCode.isSome() &&
aEvent.mCodeNameIndex == mPreviousConsumedKeyDownCode.value()) {
mPreviousConsumedKeyDownCode.reset();
}
}
// eKeyPress is a default action of eKeyDown. Therefore, eKeyPress is fired
// between eKeyDown and eKeyUp. So, received an eKeyUp for eKeyDown which
// was consumed means that following eKeyPress events should be dispatched.
// Therefore, we need to forget the fact that the preceding eKeyDown was
// consumed right now.
// NOTE: On Windows, eKeyPress may be fired without preceding eKeyDown if
// IME or utility app sends WM_CHAR message. So, if we don't forget it,
// we'd consume unrelated eKeyPress events.
else if (aEvent.mMessage == eKeyUp &&
mPreviousConsumedKeyDownCode.isSome() &&
aEvent.mCodeNameIndex == mPreviousConsumedKeyDownCode.value()) {
mPreviousConsumedKeyDownCode.reset();
}
if (localEvent.mFlags.mIsSuppressedOrDelayed) {

View File

@ -750,6 +750,7 @@ class BrowserChild final : public nsMessageManagerScriptExecutor,
RefPtr<ContentChild> mManager;
RefPtr<BrowsingContext> mBrowsingContext;
RefPtr<nsBrowserStatusFilter> mStatusFilter;
Maybe<CodeNameIndex> mPreviousConsumedKeyDownCode;
uint32_t mChromeFlags;
uint32_t mMaxTouchPoints;
layers::LayersId mLayersId;
@ -770,7 +771,6 @@ class BrowserChild final : public nsMessageManagerScriptExecutor,
bool mDidFakeShow : 1;
bool mTriedBrowserInit : 1;
bool mIgnoreKeyPressEvent : 1;
bool mHasValidInnerSize : 1;
bool mDestroyed : 1;