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

Bug 1945988 - Make PresShell::HandleEvent notify editor of mouse button events before handling them r=m_kato,smaug

Browse Source 
Chrome dispatches `compositionend` before `mousedown` when there is a
composition and user clicks somewhere.  However, we commit composition when
`EventStateManager::PostHandleEvent` moves/clears focus after `eMouseDown`
dispatching.  Therefore, our `compositionend` event is fired after `mousedown`.

The Chrome's behavior is simpler than us from mouse button event handlers point
of view because it guarantees that there is no composition when handling mouse
button events.  We should follow this behavior for better compatibility and
making the things simpler.

Differential Revision: https://phabricator.services.mozilla.com/D236827
This commit is contained in:
Masayuki Nakano
2025-02-07 22:07:16 +00:00
parent 1bad9c5e85
commit 8eb94e6791
8 changed files with 194 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
dom/base/nsFocusManager.h
View File

@@ -98,6 +98,9 @@ class nsFocusManager final : public nsIFocusManager,
* Return a focused window. Version of nsIFocusManager::GetFocusedWindow.
*/
nsPIDOMWindowOuter* GetFocusedWindow() const { return mFocusedWindow; }
static nsPIDOMWindowOuter* GetFocusedWindowStatic() {
return sInstance ? sInstance->GetFocusedWindow() : nullptr;
}
/**
* In the chrome process, retrieves the BrowsingContext corresponding

9
dom/events/TextComposition.h
View File

@@ -89,6 +89,10 @@ class TextComposition final {
}
return do_AddRef(mPresContext->GetRootWidget());
}
/**
* GetEditorBase() returns EditorBase pointer of mEditorBaseWeak.
*/
already_AddRefed<EditorBase> GetEditorBase() const;
// Returns the tab parent which has this composition in its remote process.
BrowserParent* GetBrowserParent() const { return mBrowserParent; }
// Returns true if the composition is started with synthesized event which
@@ -425,11 +429,6 @@ class TextComposition final {
// when DispatchCompositionEvent() is called.
bool mWasCompositionStringEmpty;
/**
* GetEditorBase() returns EditorBase pointer of mEditorBaseWeak.
*/
already_AddRefed<EditorBase> GetEditorBase() const;
/**
* HasEditor() returns true if mEditorBaseWeak holds EditorBase instance
* which is alive. Otherwise, false.

10
editor/libeditor/EditorBase.cpp
View File

@@ -4210,6 +4210,16 @@ void EditorBase::OnCompositionEnd(
NotifyEditorObservers(eNotifyEditorObserversOfEnd);
}
bool EditorBase::WillHandleMouseButtonEvent(WidgetMouseEvent& aMouseEvent) {
MOZ_ASSERT(aMouseEvent.mMessage == eMouseDown ||
aMouseEvent.mMessage == eMouseUp);
if (!mEventListener) {
return false;
}
OwningNonNull<EditorEventListener> editorEventListener(*mEventListener);
return editorEventListener->WillHandleMouseButtonEvent(aMouseEvent);
}
void EditorBase::DoAfterDoTransaction(nsITransaction* aTransaction) {
bool isTransientTransaction;
MOZ_ALWAYS_SUCCEEDS(aTransaction->GetIsTransient(&isTransientTransaction));

8
editor/libeditor/EditorBase.h
View File

@@ -424,6 +424,14 @@ class EditorBase : public nsIEditor,
return IsCopyToClipboardAllowedInternal();
}
/**
* Called before starting to handle eMouseDown or eMouseUp in PresShell.
*
* @return true if IME consumed aMouseEvent.
*/
MOZ_CAN_RUN_SCRIPT bool WillHandleMouseButtonEvent(
WidgetMouseEvent& aMouseEvent);
/**
* HandleDropEvent() is called from EditorEventListener::Drop that is handler
* of drop event.

72
editor/libeditor/EditorEventListener.cpp
View File

@@ -387,15 +387,6 @@ NS_IMETHODIMP EditorEventListener::HandleEvent(Event* aEvent) {
}
// mousedown
case eMouseDown: {
// EditorEventListener may receive (1) all mousedown, mouseup and click
// events, (2) only mousedown event or (3) only mouseup event.
// mMouseDownOrUpConsumedByIME is used only for ignoring click event if
// preceding mousedown and/or mouseup event is consumed by IME.
// Therefore, even if case #2 or case #3 occurs,
// mMouseDownOrUpConsumedByIME is true here. Therefore, we should always
// overwrite it here.
mMouseDownOrUpConsumedByIME =
NotifyIMEOfMouseButtonEvent(internalEvent->AsMouseEvent());
if (mMouseDownOrUpConsumedByIME) {
return NS_OK;
}
@@ -410,19 +401,6 @@ NS_IMETHODIMP EditorEventListener::HandleEvent(Event* aEvent) {
}
// mouseup
case eMouseUp: {
// See above comment in the eMouseDown case, first.
// This code assumes that case #1 is occuring. However, if case #3 may
// occurs after case #2 and the mousedown is consumed,
// mMouseDownOrUpConsumedByIME is true even though EditorEventListener
// has not received the preceding mousedown event of this mouseup event.
// So, mMouseDownOrUpConsumedByIME may be invalid here. However,
// this is not a matter because mMouseDownOrUpConsumedByIME is referred
// only by ePointerClick case but click event is fired only in case #1.
// So, before a click event is fired, mMouseDownOrUpConsumedByIME is
// always initialized in the eMouseDown case if it's referred.
if (NotifyIMEOfMouseButtonEvent(internalEvent->AsMouseEvent())) {
mMouseDownOrUpConsumedByIME = true;
}
if (mMouseDownOrUpConsumedByIME) {
return NS_OK;
}
@@ -769,10 +747,43 @@ nsresult EditorEventListener::PointerClick(
return NS_OK;
}
bool EditorEventListener::NotifyIMEOfMouseButtonEvent(
WidgetMouseEvent* aMouseEvent) {
MOZ_ASSERT(aMouseEvent);
bool EditorEventListener::WillHandleMouseButtonEvent(
WidgetMouseEvent& aMouseEvent) {
// EditorEventListener may receive (1) all mousedown, mouseup and click
// events, (2) only mousedown event or (3) only mouseup event.
// mMouseDownOrUpConsumedByIME is used only for ignoring click event if
// preceding mousedown and/or mouseup event is consumed by IME.
// Therefore, even if case #2 or case #3 occurs,
// mMouseDownOrUpConsumedByIME is set to true here. Therefore, we should
// always overwrite it if the event is mousedown.
if (aMouseEvent.mMessage == eMouseDown) {
mMouseDownOrUpConsumedByIME = NotifyIMEOfMouseButtonEvent(aMouseEvent);
}
// This code assumes that the case #1 usually occurs. However, if the case #3
// may occurs after the case #2 and the mousedown is consumed,
// mMouseDownOrUpConsumedByIME is true even though EditorEventListener
// has not received the preceding mousedown event of this mouseup event.
// So, mMouseDownOrUpConsumedByIME may be invalid here. However,
// this is not a matter because mMouseDownOrUpConsumedByIME is referred
// only by ePointerClick case but it is fired only in case #1.
// So, before a click event is fired, mMouseDownOrUpConsumedByIME is
// always initialized in the eMouseDown case if it's referred.
else {
MOZ_ASSERT(aMouseEvent.mMessage == eMouseUp);
if (NotifyIMEOfMouseButtonEvent(aMouseEvent)) {
mMouseDownOrUpConsumedByIME = true;
}
}
if (!mMouseDownOrUpConsumedByIME) {
// If the mouse button event is not consumed by IME, we should commit the
// composition to make the things simpler.
Unused << EnsureCommitComposition();
}
return mMouseDownOrUpConsumedByIME;
}
bool EditorEventListener::NotifyIMEOfMouseButtonEvent(
WidgetMouseEvent& aMouseEvent) {
if (!EditorHasFocus()) {
return false;
}
@@ -783,18 +794,11 @@ bool EditorEventListener::NotifyIMEOfMouseButtonEvent(
}
RefPtr<Element> focusedElement = mEditorBase->GetFocusedElement();
return IMEStateManager::OnMouseButtonEventInEditor(
*presContext, focusedElement, *aMouseEvent);
*presContext, focusedElement, aMouseEvent);
}
nsresult EditorEventListener::MouseDown(MouseEvent* aMouseEvent) {
// FYI: We don't need to check if it's already consumed here because
// we need to commit composition at mouse button operation.
// FYI: This may be called by HTMLEditorEventListener::MouseDown() even
// when the event is not acceptable for committing composition.
if (DetachedFromEditor()) {
return NS_OK;
}
Unused << EnsureCommitComposition();
MOZ_ASSERT_IF(!DetachedFromEditor(), !mEditorBase->IsIMEComposing());
return NS_OK;
}

16
editor/libeditor/EditorEventListener.h
View File

@@ -56,6 +56,14 @@ class EditorEventListener : public nsIDOMEventListener {
// nsIDOMEventListener
MOZ_CAN_RUN_SCRIPT NS_IMETHOD HandleEvent(dom::Event* aEvent) override;
/**
* Called before starting to handle eMouseDown or eMouseUp in PresShell.
*
* @return true if IME consumed aMouseEvent.
*/
MOZ_CAN_RUN_SCRIPT bool WillHandleMouseButtonEvent(
WidgetMouseEvent& aMouseEvent);
protected:
virtual ~EditorEventListener();
@@ -90,9 +98,13 @@ class EditorEventListener : public nsIDOMEventListener {
void CleanupDragDropCaret();
PresShell* GetPresShell() const;
nsPresContext* GetPresContext() const;
// Returns true if IME consumes the mouse event.
/**
* @return true if IME consumed aMouseEvent.
*/
MOZ_CAN_RUN_SCRIPT bool NotifyIMEOfMouseButtonEvent(
WidgetMouseEvent* aMouseEvent);
WidgetMouseEvent& aMouseEvent);
bool EditorHasFocus();
bool IsFileControlTextBox();
bool ShouldHandleNativeKeyBindings(WidgetKeyboardEvent* aKeyboardEvent);

49
layout/base/PresShell.cpp
View File

@@ -57,6 +57,7 @@
#include "mozilla/dom/Touch.h"
#include "mozilla/dom/TouchEvent.h"
#include "mozilla/dom/UserActivation.h"
#include "mozilla/EditorBase.h"
#include "mozilla/ErrorResult.h"
#include "mozilla/EventDispatcher.h"
#include "mozilla/EventForwards.h"
@@ -110,6 +111,7 @@
#include "mozilla/SVGFragmentIdentifier.h"
#include "mozilla/SVGObserverUtils.h"
#include "mozilla/Telemetry.h"
#include "mozilla/TextComposition.h"
#include "mozilla/TextEvents.h"
#include "mozilla/TimeStamp.h"
#include "mozilla/TouchEvents.h"
@@ -7147,6 +7149,53 @@ nsresult PresShell::HandleEvent(nsIFrame* aFrameForPresShell,
return NS_OK;
}
// If there is a composition and we got a pointing device events which may not
// impossible to continue the composition, we should notify the editor of the
// event before dispatching it. Then, composition will be commited before
// the editor loses focus. This behavior is compatible with Chrome.
// FIXME: Perhaps, we should do same thing before dispatching touch events.
switch (aGUIEvent->mMessage) {
case eMouseDown:
case eMouseUp: {
nsPIDOMWindowOuter* const focusedWindow =
nsFocusManager::GetFocusedWindowStatic();
if (!focusedWindow) {
break;
}
Document* const focusedDocument = focusedWindow->GetExtantDoc();
if (!focusedDocument) {
break;
}
nsPresContext* const focusedPresContext =
focusedDocument->GetPresContext();
if (!focusedPresContext) {
break;
}
const RefPtr<TextComposition> textComposition =
IMEStateManager::GetTextCompositionFor(focusedPresContext);
if (!textComposition) {
break;
}
// If there is a composition and it's managed by an editor, let's notify
// the editor of mouse button event. The editor commits the composition
// unless IME consumes the event.
if (RefPtr<EditorBase> editorBase = textComposition->GetEditorBase()) {
MOZ_ASSERT(aGUIEvent->AsMouseEvent());
editorBase->WillHandleMouseButtonEvent(*aGUIEvent->AsMouseEvent());
}
// Otherwise, we should commit the orphan composition instead.
else if (nsCOMPtr<nsIWidget> widget = textComposition->GetWidget()) {
textComposition->RequestToCommit(widget, false);
}
if (!CanHandleUserInputEvents(aGUIEvent)) {
return NS_OK;
}
break;
}
default:
break;
}
if (mPresContext) {
switch (aGUIEvent->mMessage) {
case eMouseMove:

68
widget/tests/window_composition_text_querycontent.xhtml
View File

@@ -7102,6 +7102,73 @@ function runCompositionWithSelectionChange() {
doTest(contenteditable, "runCompositionWithSelectionChange(contenteditable)");
}
function runCompositionWithClick() {
let events = [];
function eventHandler(aEvent) {
events.push(aEvent);
}
function stringifyEventAt(aIndex) {
if (aIndex >= events.length) {
return "<undefined>";
}
return `${events[aIndex].type}@${events[aIndex].target?.nodeName}`;
}
textarea.focus();
textarea.value = "";
input.value = "";
synthesizeCompositionChange(
{ "composition":
{ "string": "abc",
"clauses":
[
{ "length": 3, "attr": COMPOSITION_ATTR_RAW_CLAUSE }
]
},
"caret": { "start": 3, "length": 0 }
});
for (const type of ["compositionend", "input", "mousedown", "blur"]) {
window.addEventListener(type, eventHandler, {capture: true});
}
// Commit the composition with clicking outside the composition string (in
// this case, another text control).
synthesizeMouseAtCenter(input, {});
for (const type of ["compositionend", "input", "mousedown", "blur"]) {
window.removeEventListener(type, eventHandler, {capture: true});
}
let index = -1;
if (kExpectInputBeforeCompositionEnd) {
is(
stringifyEventAt(++index),
"input@textarea",
`runCompositionWithClick: events[${index}]`
);
}
is(
stringifyEventAt(++index),
"compositionend@textarea",
`runCompositionWithClick: events[${index}]`
);
is(
stringifyEventAt(++index),
"input@textarea",
`runCompositionWithClick: events[${index}]`
);
is(
stringifyEventAt(++index),
"mousedown@input",
`runCompositionWithClick: events[${index}]`
);
is(
stringifyEventAt(++index),
"blur@textarea",
`runCompositionWithClick: events[${index}]`
);
is(textarea.value, "abc", "runCompositionWithClick: Composition should be committed in <textarea> which had it");
is(input.value, "", "runCompositionWithClick: Composition should not be committed in <input> which newly gets focus");
}
function runForceCommitTest()
{
let events;
@@ -11333,6 +11400,7 @@ async function runTest()
runBug1675313Test();
runCommitCompositionWithSpaceKey();
runCompositionWithSelectionChange();
runCompositionWithClick();
runForceCommitTest();
runNestedSettingValue();
runBug811755Test();