2026022 - Make `EditorEventListener` listen `focus` event on window object in the capture phase

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 6/26-6/30))]

Currently, EditorEventListener listen all events which it needs in the system event group as handling everything as default action of the events. However, focus event is defined as fired after the focus is changed. I.e., the default handler should've already run before the focus event is dispatched.

Ideally, we need a internal path to run the default action before dispatching eFocus event or an internal event which is dispatched before eFocus. However, it requires a big change but this is a web-compat issue on the major web site. Therefore, we should make EditorEventListener listen focus event on the Window object in the capture phase for now.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Why would we need a new even before focus? Just have the listener on window's parent target in default group using capturing listener.
(Now, I don't remember the order of focusin and focus events)

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 6/26-6/30))]

Hmm, simply changing the event target which has focus/blur event listener to the window does break the initialization order. E.g., text control value is not selected automatically. I need to investigate where does what.

(In reply to Olli Pettay [:smaug][[e-mail]] from comment #1)

Why would we need a new even before focus? Just have the listener on window's parent target in default group using capturing listener.
(Now, I don't remember the order of focusin and focus events)

IIRC, the order of event listeners is the order of the addEventListener calls. I.e., if a focus event listener is added to the window before HTMLEditor is created, our focus event listener runs after the content's focus event listener or won't run due to a call of stopImmediatePropagation(). To avoid that, we need to add a trick to run our focus event listener first. Do you have an idea? Do we have an event target which runs before window's ones? Looks like yes, but I'm not sure whether it's safe.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 6/26-6/30))]

Attachment: Bug 2026022 - Make `TextEditor` and `HTMLEditor` initialize `Selection` before `focus` event is dispatched r=smaug!

Currently, editors, HTMLInputElement and HTMLTextAreaElement
initializes its selection as a default action of focus event. I.e.,
after the propagation ends. However, UI Events defines the event [1] as:

The focus MUST be given to the element before the dispatch of this
event type.

So, we need to initialize Selection before eFocus event is
dispatched.

For TextEditor, we can use EventTarget::PreHandleEvent(). Then, for
making the editor's focus handling simpler, we can directly call
EditorBase::OnFocus() from the PreHandleEvent method.

However, HTMLEditor needs to observe all focus move in the document
because it works with focused editing host, i.e., it's shared by
all editing hosts in the document. Therefore, for now, this patch makes
EditorEventListener listen to focus event at the Window. Although,
this cannot fix focus event listeners which are added before
EditorEventListener starts listening to focus. Therefore, a new
test does not pass with this patch.

1. https://w3c.github.io/uievents/#event-type-focus

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #2)

IIRC, the order of event listeners is the order of the addEventListener calls. I.e., if a focus event listener is added to the window before HTMLEditor is created, our focus event listener runs after the content's focus event listener or won't run due to a call of stopImmediatePropagation(). To avoid that, we need to add a trick to run our focus event listener first. Do you have an idea? Do we have an event target which runs before window's ones? Looks like yes, but I'm not sure whether it's safe.

I didn't say that we should add listener to window, but window's parent in event path.
https://searchfox.org/firefox-main/rev/187dc6baf299450446944dc90ba2239913004ba8/dom/base/nsPIDOMWindow.h#856

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 6/26-6/30))]

(In reply to Olli Pettay [:smaug][[e-mail]] from comment #4)

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #2)

IIRC, the order of event listeners is the order of the addEventListener calls. I.e., if a focus event listener is added to the window before HTMLEditor is created, our focus event listener runs after the content's focus event listener or won't run due to a call of stopImmediatePropagation(). To avoid that, we need to add a trick to run our focus event listener first. Do you have an idea? Do we have an event target which runs before window's ones? Looks like yes, but I'm not sure whether it's safe.

I didn't say that we should add listener to window, but window's parent in event path.
https://searchfox.org/firefox-main/rev/187dc6baf299450446944dc90ba2239913004ba8/dom/base/nsPIDOMWindow.h#856

Thanks! It makes my patch work better!

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 6/26-6/30))]

Hmm, some tests might depend on a buggy behavior of HTMLEditor. Currently, focus event is handled by HTMLEditor and TextEditor in this order if a text control element in an editing host gets focus. Then, HTMLEditor should do nothing but handles it and change Selection temporarily. However, we should not do that and my change required to fix it because of the focus event handling order is changed.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/52def4ebef0f
https://hg.mozilla.org/integration/autoland/rev/ea48d7fa55a9
Make `TextEditor` and `HTMLEditor` initialize `Selection` before `focus` event is dispatched r=smaug

Comment 8

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/ea48d7fa55a9

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit BugBot documentation.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:masayuki, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox152 to wontfix.

For more information, please visit BugBot documentation.

Comment 11

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/60215 for changes under testing/web-platform/tests

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 6/26-6/30))]

Although we could uplift the patch and the follow-ups in bug 2031894 to beta from the marketing point of view. However, I'd mark this as WONTFIX in beta 152 for now to conform to the theory. Feel free to ask me again if we should fix the TikTok issue ASAP.

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 14

[Alex Schmitz]

This change is causing problems in Thunderbird where the cursor will not move. It does not happen every where. The case that causes it is where we have a dialog with a form in it that has autofocus on the first input. You can change focus and type in other inputs but the visible cursor will never move out of that input. You can move it within the input but not to a new input. clicking in other inputs moves focus and typing works.

You can move the visual cursor by putting focus into the new input, switch to a different application, and then switch back, now the cursor is stuck in this new input until repeating the same process.