Closed Bug 1334981 Opened 8 years ago Closed 6 years ago

If focusmanager.testmode is true, RecentWindow.getMostRecentBrowserWindow() always returns the last opened window

Categories

(Testing :: General, defect)

Version 3
defect
Not set
normal

Tracking

(firefox-esr52 wontfix, firefox-esr60 wontfix, firefox60 wontfix, firefox61 fixed, firefox62 fixed)

RESOLVED FIXED
mozilla62
Tracking Status
firefox-esr52 --- wontfix
firefox-esr60 --- wontfix
firefox60 --- wontfix
firefox61 --- fixed
firefox62 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

I have seen this behavior while working on a Marionette test for focus handling when switching between windows (bug 1124604). So what's happening? For Marionette tests the above preference is always turned on. Now I want to test a scenario when opening a window in the background. This is happening with window.open() and a blur() on the new window, or focus() on the former window. With the preference set you won't actually see a focus change, and that's expected. But when you now want to retrieve the most recent browser window, I always get the newly opened window which is in the background. This happens even when I manually bring the former window into front. Neil, I don't think that this behavior is correct, and that getMostRecentBrowserWindow() should always return the chrome window which is indeed the top-most. What do you think?
Flags: needinfo?(enndeakin)
I think this conflicts with the change in 887718. Does it work with that patch removed?
Flags: needinfo?(enndeakin)
(In reply to Neil Deakin from comment #1) > I think this conflicts with the change in 887718. Does it work with that > patch removed? Yes, it does! Another regression as caused by that patch actually is that it is not possible right now to switch between windows via the "Window" menu. The current window never changes, even selecting a different one from the list. I assume that this is not correct, and that we have to revert the change from bug 887718. Sorry, that it has been taken that long to respond. I will push a try build with the change backed out. Maybe nothing major breaks and we can easily revert.
Blocks: 887718, 1398111
Flags: needinfo?(enndeakin)
Keywords: regression
The try build so far looks fine. For some failures I retriggered the jobs and I will wait for those, but nothing should block a review here.
Flags: needinfo?(enndeakin)
Attachment #8984131 - Flags: review?(enndeakin)
The try build was nonsense given that I pushed an artifact build :( Just figured out when I tried to test locally and switching windows still doesn't work with a downloaded build. I will have to trigger again, and I assume a subset of tests would be enough.
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Attachment #8984131 - Flags: review?(enndeakin) → review+
Hm, the r+ is not available in mozreview so I cannot land myself. Can someone please land the patch? Thanks.
:Neil: could you please grant review in mozreview tool too?
Flags: needinfo?(gbrown)
Flags: needinfo?(enndeakin)
Comment on attachment 8984131 [details] Bug 1334981 - Backed out changeset d6ca24ba3673 for regression caused by bug 887718. https://reviewboard.mozilla.org/r/249954/#review256462
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1f16a2ff45ab Backed out changeset d6ca24ba3673 for regression caused by bug 887718. r=enndeakin+6102
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Flags: needinfo?(enndeakin)
Comment on attachment 8984131 [details] Bug 1334981 - Backed out changeset d6ca24ba3673 for regression caused by bug 887718. Approval Request Comment [Feature/Bug causing the regression]: Bug 887718 which landed for Firefox 25 [User impact if declined]: Users won't be able to switch between different windows. It only happens when the preference "focusmanager.testmode" is set to "true", which is not the default. Also this is usually only used in automation. [Is this code covered by automated tests?]: Not yet, but bug 1398111 will update a test. [Has the fix been verified in Nightly?]: Yes, manually [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: Visibility of the window when switching is set everytime now, and not only outside of the test mode. [String changes made/needed]: No
Flags: needinfo?(gbrown)
Attachment #8984131 - Flags: approval-mozilla-beta?
Comment on attachment 8984131 [details] Bug 1334981 - Backed out changeset d6ca24ba3673 for regression caused by bug 887718. Fix for code behind a non-default pref which helps testing. Approved for 61.0b13.
Attachment #8984131 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Whiteboard: [checkin-needed-beta]
No longer blocks: 887718
Regressed by: 887718
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: