Closed Bug 1235822 Opened 4 years ago Closed 3 years ago

unfocused windows don't get focus when attempting exiting firefox and windows have tabs using onbeforeunload events to prevent closing them

Categories

(Firefox :: Tabbed Browser, defect, minor)

43 Branch
defect
Not set
minor

Tracking

()

RESOLVED FIXED
Firefox 53
Tracking Status
firefox53 --- fixed

People

(Reporter: diegodd88, Assigned: ke5trel)

Details

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0
Build ID: 20151221130713

Steps to reproduce:

1. Open a firefox window (window 'A') and load any page.
2. Open ANOTHER firefox window (window 'B') with at least two tabs, one including a page with "close preventing" validation (e.g. pages using onbeforeunload event to prevent closing the page). An example is https://regex101.com/ Make changes on such page so it would prevent it from closing. Focus on another tab of this window.
3. Focus the first window 'A'.
4. Attempt to close firefox, e.g. via Firefox menu -> Exit.



Actual results:

Firefox doesn't close, and no actual reason is apparent because Firefox doesn't change focus to the other window (B) with the preventing closing tab.


Expected results:

Firefox should have focused on the window B, with the tab preventing closing getting focus, so the user is aware that they can't close the browser until confirming the action in such tab and window.

Note that the tab itself DOES get focused within its window. But the window itself doesn't get focus if another window has focus.
Severity: normal → minor
Status: UNCONFIRMED → NEW
Component: Untriaged → DOM: Events
Ever confirmed: true
Product: Firefox → Core
Summary: unfocused windows don't get focus when attempting exiting firefox if such windows have tabs using onbeforeunload events to prevent closing them → unfocused windows don't get focus when attempting exiting firefox and windows have tabs using onbeforeunload events to prevent closing them
Whiteboard: [DUPEME]
This patch makes the beforeunload dialog get window focus so it is seen as long as an application window is active (so as not to steal focus from other applications). The dialog already steals tab focus so it should also be able to steal window focus. The existing shutdown test has been modified slightly to make sure that the dialog's window and tab get focused.

I will need someone to do a try push for me.
Attachment #8806281 - Flags: review?(mak77)
Comment on attachment 8806281 [details] [diff] [review]
bug1235822.patch - Focus window for beforeunload dialog so it is seen when shutting down

Review of attachment 8806281 [details] [diff] [review]:
-----------------------------------------------------------------

This bug should be moved to the Tabbed Browser component.
Attachment #8806281 - Flags: review?(mak77) → review?(dao+bmo)
Component: DOM: Events → Tabbed Browser
Product: Core → Firefox
Sorry I missed the request :(
On the other side, I think Dao is a better reviewer for tabbed browser code.
The patch itself looks ok to me, but I'm not sure of possible consequences of this focus change. But it also looks like an easy backout in case of troubles.
Comment on attachment 8806281 [details] [diff] [review]
bug1235822.patch - Focus window for beforeunload dialog so it is seen when shutting down

>+          // Focus window for beforeunload dialog so it is seen but don't
>+          // steal focus from other applications.
>+          if (event.detail && event.detail.tabPrompt &&
>+              event.detail.inPermitUnload &&
>+              Services.focus.activeWindow)
>+            window.focus();

nit: please break the line after 'event.detail &&'

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e2cd176870f60fcc1f9b6f0bb9c3dc435143b9a8
Attachment #8806281 - Flags: review?(dao+bmo) → review+
Assignee: nobody → kestrel
OS: Unspecified → All
Hardware: Unspecified → All
Whiteboard: [DUPEME]
Thanks for the try run, seems OK, failures don't appear to be related to this patch.
Attachment #8806281 - Attachment is obsolete: true
Attachment #8811375 - Flags: review?(dao+bmo)
Comment on attachment 8811375 [details] [diff] [review]
bug1235822v2.patch - Fixed nit

Thanks!
Attachment #8811375 - Flags: review?(dao+bmo) → review+
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/51357859be9e
Focus window for beforeunload dialog so it is seen when shutting down. r=dao
https://hg.mozilla.org/mozilla-central/rev/51357859be9e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in before you can comment on or make changes to this bug.