Closed Bug 846365 Opened 12 years ago Closed 12 years ago

Change - New window opens in desktop Firefox when closing Metro Firefox

Categories

(Firefox for Metro Graveyard :: Shell, defect, P1)

defect

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 22

People

(Reporter: josh.tumath+bugzilla, Assigned: bbondy)

References

Details

(Whiteboard: feature=change c=Opening_and_closing u=metro_firefox_user p=3 status=verified)

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:22.0) Gecko/20130228 Firefox/22.0 Build ID: 20130228030852 Steps to reproduce: 1. Open desktop Firefox. 2. Browse to any Web site. 3. Open Metro Firefox. 4. Browse to any Web site. 5. Go back to the desktop with the desktop Firefox window still active. 6. Close Metro Firefox from the app list by doing the following: a. Move your mouse to the very top left corner of the screen. b. Slide the mouse down the edge of the screen. c. Right click on the Metro Firefox app in the app list. d. Click on "Close" in the context menu. Actual results: A new window appeared in Firefox with just an empty tab.
I always see this and it bothers me, confirming.
Status: UNCONFIRMED → NEW
Ever confirmed: true
I'd not seen this before. Are the steps outlined the only way to trigger this bug?
(In reply to Asa Dotzler [:asa] from comment #2) > I'd not seen this before. Are the steps outlined the only way to trigger > this bug? As far as I'm aware, yes. I can only reproduce it by closing the app via the app list.
I get this when I bring up the application list on the right windows edge (the bar that comes up when you window + shift + tab) and then right click on the app and close. This is how I mostly close applications in win8 and this bug really bothers me. I can fix it in it4 if you'd like as a defect.
Also closing from the application bar leads to a unclean shutdown so it always brings up the same tabs as you last used from session restore.
Whiteboard: p=3
OK. Yes. I think we need to fix this. Marking it as a *change* to Bug 831888 - Story - Closing Firefox because we didn't have that form of application closing covered in that story.
Blocks: 831888
No longer blocks: metrov1triage
Whiteboard: p=3 → feature=change p=3
Blocks: metrov1it4
Priority: -- → P1
Summary: New window opens in desktop Firefox when closing Metro Firefox → Change - New window opens in desktop Firefox when closing Metro Firefox
Whiteboard: feature=change p=3 → feature=change c=Opening_and_closing u=metro_firefox_user p=3
Assignee: nobody → netzen
Found the reason for this but have not fixed yet. Basically if you don't close the application while it's activated we don't get the normal shutdown sequence. So Windows restarts our process with a special command line to allow us to do any normal cleanup at that point. .... so strange that they do this... > Closing background sessions and cleaning up private browsing data > There are a few user affordances that allow for a user to close a new > experience browsing session without first bringing the browser window > to the foreground. >• Pulling the thumbnail of the browser from the switch list to the > bottom of the screen. > • Right-clicking the thumbnail of the browser from the switch list > and choosing ‘close’. > • Pulling the browser from the back stack using the left edge > and dragging the window to the bottom of the screen. > In such instances, the browser can’t run code before the app is closed. > The browser’s window isn’t sent a “DefaultBrowserClosing” message. > Instead, Process Lifetime Management closes the browser and immediately > reactivates it as a desktop process with the -BackgroundSessionClosed > command-line argument, e.g. “examplebrowser.exe -BackgroundSessionClosed”. Use > this opportunity to clean up data from the previous browsing session, > particularly sensitive user data that may have been persisted to disk during a > private browsing session. Don’t present a window or run other tasks. Exit as > soon as the cleanup is complete. If no data cleanup tasks are required (for > instance, if the browser doesn’t offer the private browsing feature), handle the > activation argument and exit.
I think in our case we only really want to clear the session history.
Status: NEW → ASSIGNED
Component: General → Shell
Attached patch Patch v1. (obsolete) (deleted) — Splinter Review
See Comment 7 before reviewing. This adds support for background sessions being closed which re-opens Firefox with a special commandline to do cleanup. Basically I route that command line to the WinRT widget code and do a partial initialization which is enough to get the profile directory. Then clear the session store and exit silently.
Attachment #727837 - Flags: review?(jmathies)
Attached patch Patch v1. (deleted) — Splinter Review
Sorry was missing a closing parentheses on a last minute addition
Attachment #727837 - Attachment is obsolete: true
Attachment #727837 - Flags: review?(jmathies)
Comment on attachment 727846 [details] [diff] [review] Patch v1. ack I forgot to mark r? on the newly uploaded patch.
Attachment #727846 - Flags: review?(jmathies)
Comment on attachment 727846 [details] [diff] [review] Patch v1. Review of attachment 727846 [details] [diff] [review]: ----------------------------------------------------------------- Is there anything we can do in here about those cache problems we were having? ::: widget/windows/winrt/MetroApp.cpp @@ +235,5 @@ > + > + // Perform any cleanup for unclean shutdowns here, such as when the background session > + // is closed via the appbar on the left when outside of Metro. Windows restarts the > + // process solely for cleanup reasons. > + if (IsBackgroundSessionClosedStartup() && SUCCEEDED(XRE_metroStartup(false))) { Should we check the result of XRE_metroStartup? I think we want to return from within this bock no matter what.
Attachment #727846 - Flags: review?(jmathies) → review+
(In reply to Jim Mathies [:jimm] from comment #12) > Comment on attachment 727846 [details] [diff] [review] > Patch v1. > > Review of attachment 727846 [details] [diff] [review]: > ----------------------------------------------------------------- > > Is there anything we can do in here about those cache problems we were > having? Unfortunately no. The way the disk cache is designed, if the program crashes or closes without notification, we have the potential of a header to files on disk mismatch that we can't detect. Luckily though, the disk cache is being redesigned as we speak in this project: https://wiki.mozilla.org/Necko/Cache/Plans > > ::: widget/windows/winrt/MetroApp.cpp > @@ +235,5 @@ > > + > > + // Perform any cleanup for unclean shutdowns here, such as when the background session > > + // is closed via the appbar on the left when outside of Metro. Windows restarts the > > + // process solely for cleanup reasons. > > + if (IsBackgroundSessionClosedStartup() && SUCCEEDED(XRE_metroStartup(false))) { > > Should we check the result of XRE_metroStartup? I think we want to return > from within this bock no matter what. We may want to do that if we have anything in that block in the future that doesn't require initialization, but at the moment there is no need to ignore the result. The call that it will fall through to "hr = sCoreApp->Run(sMetroApp.Get());" will simply fail right away with a COM error of element not found, and the program will exit. I think because it tries to find a CoreWindow but none exists. For that reason I'll just leave this as is. Thanks for the weekend review!
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Flags: needinfo?(jbecerra)
QA Contact: jbecerra
Tested on 2013-03-29 on a nightly built from http://hg.mozilla.org/mozilla-central/rev/8693d1d4c86d - Tested with the steps in comment #0 when there was only desktop and metro Firefox were the only two applications running as well as when there were other apps running. - Reopening metro Firefox in Metro mode opens cleanly with a Start page if I have set the preference to open with a start page, or it opens with the previous tabs from last time if I have that preference set. - I couldn't close desktop Firefox using the same steps, but maybe that's intended? Filed a defect bug 856097.
Status: RESOLVED → VERIFIED
Flags: needinfo?(jbecerra)
Whiteboard: feature=change c=Opening_and_closing u=metro_firefox_user p=3 → feature=change c=Opening_and_closing u=metro_firefox_user p=3 status=verified
Depends on: 856097
No longer depends on: 856097
Went through the following "Change" for iteration #7 testing without any issues. Used the following build: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-05-25-06-25-25-mozilla-central/ - Went through the original steps outlined in Comment 0 without any issues - Went through the test cases added in Comment 16 without any issues
User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0 Build ID: 20130825030201 Built from http://hg.mozilla.org/mozilla-central/rev/01576441bdc6 WFM Tested on windows 8 using latest nightly for iteration-12. Followed steps provided in comment0 and got expected result.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: