1653160 - focus handling seems to be broken in fission window

Status: RESOLVED FIXED

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

Description

[Hiroyuki Ikezoe (:hiro)]

Attachment: t.html

I am bit confused, it may not be a problem at all.

Steps to reproduce

1. Open https://hsivonen.fi/fission-host.html
2. Click somethere in one of iframes there
3. You will see a "iframe [object Window] focus"
4. Click again somewhere in the same iframe
5. You will see a pair of "iframe [object Window] blur" and "iframe [object Window] focus"

In non Fission window, you will never see the pair of "blur" and "focus".

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Sorry the attachment in comment 0 is not necessary at all. I was initially thinking the issue happens only in nested iframes.

Comment 2

[Neha Kochar [:neha]]

Henri is looking into Fission focus and bug 1613054 may be related.

Comment 3

[Henri Sivonen (:hsivonen)]

Preliminary guess: Upon click, we always try to "raise" the PuppetWidget window of the OOP iframe even if it already has focus.

It looks like we need some check that makes sense for OOP iframes here, where we even have a comment hinting at a problem:
https://searchfox.org/mozilla-central/rev/d54210d490ef335b13fc1fcac817525120c8c46b/dom/base/nsFocusManager.cpp#2738

The obvious solution is checking against the focused browsing context, but I need to think more carefully about the case where the OOP iframe tree is deeper.

Comment 4

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1653160 - Avoid re-raising the PuppetWidget of an already-focused OOP iframe upon click.

Comment 5

[Henri Sivonen (:hsivonen)]

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

Comment 6

[Henri Sivonen (:hsivonen)]

(In reply to Neha Kochar [:neha] from comment #2)

Henri is looking into Fission focus and bug 1613054 may be related.

At this point, it looks like this in unrelated to bug 1613054.

Comment 7

[Henri Sivonen (:hsivonen)]

The currently-attached patch leaves the observable order of the click and focus events in the OOP iframe reversed compared to the non-Fission case. Let's leave potentially changing that as a follow-up.

Comment 8

[Henri Sivonen (:hsivonen)]

(In reply to Henri Sivonen (:hsivonen) from comment #7)

The currently-attanche patch leaves the observable order of the click and focus events in the OOP iframe reversed compared to the non-Fission case. Let's leave potentially changing that as a follow-up.

Filed as bug 1660805 (bug not introduced by the fix here).

Comment 9

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1653160 test - Test clicking and already-focused OOP iframe and test moving focus between nested OOP iframes.

Comment 10

[Henri Sivonen (:hsivonen)]

(In reply to Henri Sivonen (:hsivonen) from comment #3)

I need to think more carefully about the case where the OOP iframe tree is deeper.

Per discussion with Neha, spinning this off into a follow-up: bug 1661227.

Comment 11

[Henri Sivonen (:hsivonen)]

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

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b162b405a551
Avoid re-raising the PuppetWidget of an already-focused OOP iframe upon click. r=nika
https://hg.mozilla.org/integration/autoland/rev/998f1372ebe0
test - Test clicking and already-focused OOP iframe and test moving focus between nested OOP iframes. r=nika

Comment 13

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/b162b405a551
https://hg.mozilla.org/mozilla-central/rev/998f1372ebe0