Closed
Bug 1206559
Opened 10 years ago
Closed 10 years ago
[e10s] window.focus() in child frame doesn't redirect raise window request to parent process
Categories
(Testing :: Mochitest, defect)
Testing
Mochitest
Tracking
(firefox44 fixed)
RESOLVED
FIXED
mozilla44
Tracking | Status | |
---|---|---|
firefox44 | --- | fixed |
People
(Reporter: xidorn, Assigned: enndeakin)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
3.34 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
It seems to be unreliable to use waitForFocus in content process. Tests which rely on getting focus via this function is usually stuck after "must wait for focus".
Reporter | ||
Updated•10 years ago
|
Summary: waitForFocus doesn't work reliably on content process → waitForFocus doesn't work reliably in content process
Reporter | ||
Comment 1•10 years ago
|
||
A very simple example of this issue is layout/forms/test/test_bug564115.html which uses waitForFocus for switching focus back and forth. It reliably gets stuck at the second waitForFocus call because no one else can focus the window for it.
Assignee | ||
Comment 2•10 years ago
|
||
It looks like nsGlobalWindow::FocusOuter doesn't handle the case where someone tries to call window.focus() on a child iframe when in a content process.
Assignee: nobody → enndeakin
Points: --- → 2
Reporter | ||
Comment 3•10 years ago
|
||
It seems adding
> aMessage.target.ownerDocument.defaultView.focus();
to the handler of "SpecialPowers.Focus" message in SpecialPowersObserver works.
Assignee | ||
Comment 4•10 years ago
|
||
Reporter | ||
Comment 5•10 years ago
|
||
It seems there are some other issues make waitForFocus not reliable in content process... I cannot make my test work with the proposed patch. (And the approach I figured out in comment 3 doesn't work now for me as well)
It seems even if the window is raised to the top, it is still possible that the content doesn't get the focus event.
Assignee | ||
Comment 6•10 years ago
|
||
A try run with the patch and enabled for e10s worked ok:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=65ab7198a94a
Assignee | ||
Updated•10 years ago
|
Attachment #8663680 -
Flags: review?(bugs)
Comment 7•10 years ago
|
||
Comment on attachment 8663680 [details] [diff] [review]
Send raise request to parent
Don't we rather want to fix nsPuppetWidget
http://mxr.mozilla.org/mozilla-central/source/widget/PuppetWidget.cpp?rev=b96e95b7b4b9&mark=258-263#258
And simplify cases like SetActiveWindow handling in
nsGlobalWindow::FocusOuter (Or perhaps SetActiveWindow should do something only in parent process, though, nsGlobalWindow::FocusOuter does call either
SetActiveWindow or SendRequestFocus).
Feel free to re-ask review if you think your approach is better, but please explain why. (I'd like to push as much as possible the content process specific stuff to nsPuppetWidget)
Attachment #8663680 -
Flags: review?(bugs) → review-
Assignee | ||
Comment 8•10 years ago
|
||
Attachment #8663680 -
Attachment is obsolete: true
Attachment #8667030 -
Flags: review?(bugs)
Comment 9•10 years ago
|
||
Comment on attachment 8667030 [details] [diff] [review]
Better fix
RaiseWindow isn't used often and this seem to make it work rather nicely.
I think I'd null check mTabChild (since proving it can't be null when SetFocus is called is a bit hard).
So
if (aRaise && mTabChild) {
mTabChild->SendRequestFocus(true);
}
Attachment #8667030 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 11•10 years ago
|
||
Assignee | ||
Comment 12•10 years ago
|
||
Assignee | ||
Comment 13•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/5a67c866d14d07921d6f35e22f45b78846d9500b
Bug 1206559, forward PuppetWidget::SetFocus request to the parent process, r=smaug
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox44:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Reporter | ||
Updated•10 years ago
|
Flags: needinfo?(enndeakin)
Assignee | ||
Updated•10 years ago
|
Summary: waitForFocus doesn't work reliably in content process → [e10s] window.focus() in child frame doesn't redirect raise window request to parent process
You need to log in
before you can comment on or make changes to this bug.
Description
•