1674702 - Additional pair of blur and focus event gets fired while calling window.focus() on OOP iframe

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P3)

Description

[Edgar Chen [:edgar]]

STR:

1. enable fission
2. load https://codepen.io/edgarchen-the-decoder/full/xxOjjGX
3. click window.focus() button to trigger window.focus() call

Expected result:
No additional blur and focus event.

Actual result:
Additional blur and focus event gets fired.

Comment 1

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=71dd872e30d6e38f8796f9ae8e7e1e3b3408750f

Comment 2

[Edgar Chen [:edgar]]

One case of the additional focus event gets fired is from nsFocusManager::Focus call in nsFocusManager::WindowRaised where we always pass true as aNewDocument.

Comment 3

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/jobs?repo=try&group_state=expanded&revision=f416e647e29b72afe1a737436ed02d5d63ab9195

Comment 4

[Edgar Chen [:edgar]]

Attachment: Bug 1674702 - Part 1: Use Element in nsFocusManager::Blur and ::Focus;

Comment 5

[Edgar Chen [:edgar]]

Attachment: Bug 1674702 - Part 2: Compute focus-in-other-content-process flag in nsFocusManager::Focus;

Depends on D96719

Comment 6

[Edgar Chen [:edgar]]

(Note to myself: rebase over bug 1618639)

Comment 7

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=589854282f090b9f13049422414c6f83f84404f4

Comment 8

[Edgar Chen [:edgar]]

I should update what I found, so basically I found following things that we should think about how to handle them,

• comment #2, this is handled in bug 1674743.
• Blur will be always redirected to focusedBrowsingContext's process if focusedBrowsingContext is in other process, this will lead to an unnecessary blur action on focusedBrowsingContext's process, this is why we see additional blur events on OOP iframe for this bug. And it doesn't run Blur steps in current process, I guess we should? At least for https://searchfox.org/mozilla-central/rev/96e2c6e14998f38e419850d55d8a3d32a3fc244a/dom/base/nsFocusManager.cpp#2085-2088, but not sure if there are other possible cases.
• Calling SetFocusedWindowInternal will also update focusedBrowsingContext, this could trigger unnecessary updating, I see one case that focusedBrowsingContext being updated to the framer, so that OOP iframe think it should run focus steps again.

Comment 9

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=40ab87a2aa13b7da2d0a5940fa77278af057891c

Comment 10

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e199a337a591c9da0e3f43cbc5cb8449265638b9

Comment 11

[Edgar Chen [:edgar]]

Attachment: Bug 1674702 - Do not bounce IPC message to framer if focus is handled synchronously in OOP iframe;

This suppress unnecessary blur and focus actions in the framer, we would need
to find a way to notify framer that the focus has moved to OOP iframe to
dispatch corresponding blur event, see bug 1677474.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15ceb99908ca
Do not bounce IPC message to framer if focus is handled synchronously in OOP iframe; r=hsivonen

Comment 13

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

Failed to create upstream wpt PR due to merge conflicts. This requires fixup from a wpt sync admin.

Comment 14

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/15ceb99908ca

Comment 15

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

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

Comment 16

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

Upstream PR merged by moz-wptsync-bot