Focus events fire in both parent and child frames when raising windows with fission enabled.
Categories
(Core :: DOM: UI Events & Focus Handling, defect)
Tracking
()
People
(Reporter: enndeakin, Assigned: hsivonen)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
- Enable fission preference. (fission.autostart)
- Have a page with a child frame in another process. The child frame should contain an input element.
- Focus the input element.
- Lower the window and raise it again.
- The input should be focused (once 1617788 is complete), however focus events fire on both the parent frame and child frame.
I will try to create a test case soon.
The issue I think occurs in WindowRaised:
The call to GetFocusedDescendant wants to get the lowest most frame and then call Focus() on it. However when a descendant is out of process, the focus attempt occurs in a parent frame. But also, the descendant process does the same, resulting in two focus attempts.
Perhaps it is sufficient to simply skip this part in the ancestor process when we know a child process will handle focusing.
|
||
Comment 1•4 years ago
|
||
Henri, what is the expected behavior in this case? Do we want to fire focus events in both parent and child frames?
|
||
Updated•4 years ago
|
|
Assignee | |
Comment 2•4 years ago
|
||
I added console.log calls to https://hsivonen.fi/fission-host.html , and the results with the patch are indeed very wrong. Curiously, despite the OOP iframe restoring focus for the purpose of focus ring and text insertion point, the Web-visible events fire only in the wrong place as opposed to both the right and the wrong place as comment 0 says.
|
|
Assignee | |
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
|
|
Assignee | |
Comment 4•4 years ago
|
||
|
|
Assignee | |
Comment 5•4 years ago
|
||
The attached patch avoids firing the events at top-level Web content. Pernosco suggests that there is an attempt to fire the events within the OOP iframe, but I don't see them in the Web Console.
|
|
Assignee | |
Comment 6•4 years ago
|
||
(In reply to Henri Sivonen (:hsivonen) from comment #5)
The attached patch avoids firing the events at top-level Web content. Pernosco suggests that there is an attempt to fire the events within the OOP iframe, but I don't see them in the Web Console.
Looks like console.log from OOP iframes is broken. I think this patch now fixes the bug. The new test fails without the patch in Fission and succeeds in Fission with the patch and succeeds in Chrome.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c9647a9b398f45214fc623e15c9c7c58cb20b066
|
|
Assignee | |
Comment 7•4 years ago
|
||
Weird. The above try run had a failure related to the harness processing a task I didn't even request.
Let's try again:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8a34221cc9f8c59b69b3ae82dca47676d4407d17
|
|
Assignee | |
Comment 8•4 years ago
|
||
Let's see if a rebase makes the try harness failure go away:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f12ecc1f3ab368b8343a9dfbf010856db9dbcb50
|
|
Assignee | |
Comment 9•4 years ago
|
||
|
|
Assignee | |
Comment 10•4 years ago
|
||
Trying an even longer wait time for the new test:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0357a6e7d89954b8777145840802f7c175a4ab5a
|
|
Assignee | |
Comment 11•4 years ago
|
||
It appears that the patch itself now works and the test works locally, but on try, debug builds run so slowly that the test doesn't wait for long enough. Trying even longer wait times even though such long waits are not nice to have in CI:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7fd46436876959d2b897324f3b5e6336eae550b1
(One option would be looking into making the test run for opt builds only.)
|
|
Assignee | |
Comment 12•4 years ago
|
||
Let's see if opt builds do better on try in terms of timing:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=53f6c4bc57192f61891c6a3c5419243611ffb4bf
|
|
Assignee | |
Comment 13•4 years ago
|
||
The test I'm adding here works for me locally on Linux (I wrote the test on Linux). However, it fails in CI on Linux but works on Mac and Windows in CI.
To make progress, I'm disabling the test on Linux in CI:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e49a8375e73b46a6addb7bddc5e8ab62d9e48aa8
Filed bug 1642889 as a follow-up.
|
|
Assignee | |
Comment 14•4 years ago
|
||
Broader try run in order to avoid surprises:
https://hg.mozilla.org/try/rev/d671e6a961ff4e730065a72b34649409a5082f1b
|
||
Comment 15•4 years ago
|
||
Pushed by hsivonen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/01418842d5c7 Avoid firing focus on iframe when raising window if actual focus out-of-process. r=NeilDeakin
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/24007 for changes under testing/web-platform/tests
Can't merge web-platform-tests PR due to failing upstream checks: Github PR https://github.com/web-platform-tests/wpt/pull/24007 * Community-TC (pull_request) (https://community-tc.services.mozilla.com/tasks/groups/ZIbTXUPwTwuWteBzqQH7Pw)
|
|
Assignee | |
Comment 18•4 years ago
|
||
(In reply to Web Platform Test Sync Bot (Matrix: #interop:mozilla.org) from comment #17)
Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/24007
- Community-TC (pull_request)
(https://community-tc.services.mozilla.com/tasks/groups/
ZIbTXUPwTwuWteBzqQH7Pw)
Commented in the GitHub PR.
|
||
Comment 19•4 years ago
|
||
| bugherder | ||
|
||
Comment 20•4 years ago
|
||
The patch landed in nightly and beta is affected.
:hsivonen, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.
For more information, please visit auto_nag documentation.
|
|
Assignee | |
Comment 21•4 years ago
|
||
(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #20)
The patch landed in nightly and beta is affected.
:hsivonen, is this bug important enough to require an uplift?
If not please setstatus_betatowontfix.For more information, please visit auto_nag documentation.
Setting wontfix, because this bug is Fission-specific and Fission isn't enabled in beta.
|
|
Assignee | |
Updated•4 years ago
|
|
|
Assignee | |
Comment 22•4 years ago
|
||
Corrected to disabled instead of wontfix.
Can't merge web-platform-tests PR due to failing upstream checks: Github PR https://github.com/web-platform-tests/wpt/pull/24007 * Community-TC (pull_request) (https://community-tc.services.mozilla.com/tasks/groups/PYwBzBQaQ9S3DnWD_qMkww)
Can't merge web-platform-tests PR due to failing upstream checks: Github PR https://github.com/web-platform-tests/wpt/pull/24007 * Community-TC (pull_request) (https://community-tc.services.mozilla.com/tasks/groups/buf0UsS2Q9uj32YFNjytSA)
Upstream PR merged by jgraham
Description
•