Bug 1709125 Comment 48 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Neil Deakin from comment #47)
> waitForLoadAndFocusOnWindow is used to focus the desiredWindow that is passed to it. But the actual focus might be in a subframe (perhaps nested several levels deep)
> 
> Let's say some one focused an element in a nested subframe within a window, and assume for now that the window and all subframes are all in the same process. Then, one switches to a different window. When one switches back to that window again using waitForFocus, waitForLoadAndFocusOnWindow will be called with the top-level window as the desiredWindow. However, the focused window would be set to the nested subframe, and that is where the window/document focus events would be fired. So getFocusedElementForWindow is used to get this nested subframe and listen for events on it.

When the top `BrowsingContext` of a given hierarchy isn't the active `BrowsingContext`, we don't have the information about what the focused `BrowsingContext` of that hierarchy would be in processes other than the one hosting the corresponding DOM window.

That is, we only know about the actually-focused `BrowsingContext` in a cross-process way.

Even if we did know what the would-be-focused OOP `BrowsingContext` is for a given currently-inactive hierarchy, it's not clear to me if we have a mechanism that would allow for a JS-based `onfocus` handler on it.

> From brief testing I just did, it looks like the events only fire on the lowest nested process and frame that is focused. It seems from the description above (I haven't tested) that focusedWindow is null in a document containing a iframe when that iframe is focused and out-of-process?

I believe so.

> However testing in Chrome shows that document.hasFocus() returns true if any descendant iframe has focus. (so the HasFocus changes in this patch look correct from a brief glance).
> 
> Do we want to simply deprecate using focusedWindow and instead rely on using the focused browsing context instead?

The problem is: What would be compare it with? The problem isn't so much switching `focusedWindow` to a Fission-aware API but the window out param of `getFocusedElementForWindow`.

Do you have ideas of where to go from here?

> Where is focusedWindow being accessed where the test failure happens?

From https://searchfox.org/mozilla-central/rev/5359952d8b0be3e706e8c943c2bef2674723b8a9/testing/mochitest/tests/SimpleTest/SimpleTest.js#994 called from https://searchfox.org/mozilla-central/rev/5359952d8b0be3e706e8c943c2bef2674723b8a9/testing/mochitest/tests/SimpleTest/SimpleTest.js#1087-1091 AFAICT.

> The SpecialPowers.focus() probably isn't needed, unless window.focus() runs into priviledge problems maybe.

https://searchfox.org/mozilla-central/rev/5359952d8b0be3e706e8c943c2bef2674723b8a9/testing/mochitest/tests/SimpleTest/SimpleTest.js#1098 is needed per comment 41. However, changing `desiredChildWindow` to `desiredWindow` seems to work.

AFAICT, `desiredChildWindow` is needed for the `addEventListener` line:
https://searchfox.org/mozilla-central/rev/5359952d8b0be3e706e8c943c2bef2674723b8a9/testing/mochitest/tests/SimpleTest/SimpleTest.js#1094
...and I don't know if we have any Fission-compatible way to accomplish the same thing.
(In reply to Neil Deakin from comment #47)
> waitForLoadAndFocusOnWindow is used to focus the desiredWindow that is passed to it. But the actual focus might be in a subframe (perhaps nested several levels deep)
> 
> Let's say some one focused an element in a nested subframe within a window, and assume for now that the window and all subframes are all in the same process. Then, one switches to a different window. When one switches back to that window again using waitForFocus, waitForLoadAndFocusOnWindow will be called with the top-level window as the desiredWindow. However, the focused window would be set to the nested subframe, and that is where the window/document focus events would be fired. So getFocusedElementForWindow is used to get this nested subframe and listen for events on it.

When the top `BrowsingContext` of a given hierarchy isn't the active `BrowsingContext`, we don't have the information about what the focused `BrowsingContext` of that hierarchy would be in processes other than the one hosting the corresponding DOM window.

That is, we only know about the actually-focused `BrowsingContext` in a cross-process way.

Even if we did know what the would-be-focused OOP `BrowsingContext` is for a given currently-inactive hierarchy, it's not clear to me if we have a mechanism that would allow for a JS-based `onfocus` handler on it.

> From brief testing I just did, it looks like the events only fire on the lowest nested process and frame that is focused. It seems from the description above (I haven't tested) that focusedWindow is null in a document containing a iframe when that iframe is focused and out-of-process?

I believe so.

> However testing in Chrome shows that document.hasFocus() returns true if any descendant iframe has focus. (so the HasFocus changes in this patch look correct from a brief glance).
> 
> Do we want to simply deprecate using focusedWindow and instead rely on using the focused browsing context instead?

The problem is: What would we compare it with? The problem isn't so much switching `focusedWindow` to a Fission-aware API but the window out param of `getFocusedElementForWindow`.

Do you have ideas of where to go from here?

> Where is focusedWindow being accessed where the test failure happens?

From https://searchfox.org/mozilla-central/rev/5359952d8b0be3e706e8c943c2bef2674723b8a9/testing/mochitest/tests/SimpleTest/SimpleTest.js#994 called from https://searchfox.org/mozilla-central/rev/5359952d8b0be3e706e8c943c2bef2674723b8a9/testing/mochitest/tests/SimpleTest/SimpleTest.js#1087-1091 AFAICT.

> The SpecialPowers.focus() probably isn't needed, unless window.focus() runs into priviledge problems maybe.

https://searchfox.org/mozilla-central/rev/5359952d8b0be3e706e8c943c2bef2674723b8a9/testing/mochitest/tests/SimpleTest/SimpleTest.js#1098 is needed per comment 41. However, changing `desiredChildWindow` to `desiredWindow` seems to work.

AFAICT, `desiredChildWindow` is needed for the `addEventListener` line:
https://searchfox.org/mozilla-central/rev/5359952d8b0be3e706e8c943c2bef2674723b8a9/testing/mochitest/tests/SimpleTest/SimpleTest.js#1094
...and I don't know if we have any Fission-compatible way to accomplish the same thing.

Back to Bug 1709125 Comment 48