Closed
Bug 368677
Opened 18 years ago
Closed 16 years ago
Sessionstore should ignore popup windows
Categories
(Firefox :: Session Restore, defect, P5)
Firefox
Session Restore
Tracking
()
VERIFIED
FIXED
Firefox 3.1b1
People
(Reporter: volkmarkostka, Assigned: zeniko)
References
(Blocks 1 open bug)
Details
(Keywords: dataloss)
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
dietrich
:
review+
|
Details | Diff | Splinter Review |
(deleted),
text/html
|
Details |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070129 Minefield/3.0a2pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070129 Minefield/3.0a2pre
Sessionstore should ignore page generated popup windows. It is simply annoying if you have forgotton that a popup is open and you closed your main window to find out that there is still a popup window. In this case your sessionstore will only contain this popup window but not the content of the main window. So you lose your complete session.
I think it is safe to assume that popup windows almost never contain important data so storing them is rather useless.
Reproducible: Always
Steps to Reproduce:
1. Visit a page where you can open a popup window e.g. http://www.mozillazine.org/forums/
2. Open the smilie popup.
3. Close the main window
4. Close the popup
5. restart firefox
Actual Results:
Only the smilie window is restored
Expected Results:
The last closed main window should be restored
Comment 1•18 years ago
|
||
I disagree that it is safe to assume that. I think in reality the ease of restoring multiple windows needs to be improved so that both the popup window and main window are restored.
The most common use case for closing multiple windows is closing them one at a time which sessionstore doesn't support, whereas it does handle closing them all at once with file - exit.
Assignee | ||
Comment 4•18 years ago
|
||
Attachment #265837 -
Flags: review?(dietrich)
Assignee | ||
Updated•18 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-firefox3?
OS: Windows XP → All
Hardware: PC → All
Comment 5•18 years ago
|
||
Blocking since we need to figure out what the right thing is here, and it might be something about handling session saving better to avoid the bad use case illustrated.
Flags: blocking-firefox3? → blocking-firefox3+
Comment 6•18 years ago
|
||
Just to throw in a thought here, possibly something slightly different. I use a Mac and I'm forever closing the window, which of course doesn't quit the app. So when the app is quitted and restarted as far as it is concerned there was nothing open previously so doesn't restore.
Assignee | ||
Comment 7•18 years ago
|
||
(In reply to comment #6)
> possibly something slightly different.
Slightly enough that it doesn't have much to do with this bug at all. Please file a new one... so that we can introduce a pref browser.sessionstore.allow_empty_session or something to disable what we're currently doing (resp. what bug 354894 will even improve).
Assignee | ||
Comment 8•18 years ago
|
||
(In reply to comment #6)
> So when the app is quitted and restarted as far as it is concerned there was
> nothing open previously so doesn't restore.
Filed bug 382006 to that end.
Comment 9•18 years ago
|
||
(In reply to comment #8)
> (In reply to comment #6)
> > So when the app is quitted and restarted as far as it is concerned there was
> > nothing open previously so doesn't restore.
>
> Filed bug 382006 to that end.
I'm afraid you've misunderstood me, and apparently what I was after already does work, I was mistakenly thinking of something else.
Comment 10•18 years ago
|
||
So, the big problem I think we need to solve is (on Windows) how to handle the "I closed my app window, and found a lurking popup/Download Manager that trashed my session" case. I'm not sure whether this helps here, or not.
Assignee | ||
Comment 11•18 years ago
|
||
(In reply to comment #10)
> "I closed my app window, and found a lurking popup/Download Manager that
> trashed my session" case. I'm not sure whether this helps here, or not.
The suggested solution (cf. comment #4) is to restore any popup together with the last closed app window (or only the app window, although then you might lose data the other way round). We already do that in case you've missed the Download Manager (which currently is only involved in bug 354894).
So at worst a user would have to close a popup twice (when closing down and after restoring the session).
An alternative would be to offer a Recently Closed Windows list, although we'd have to place it in a way that users will actually find it...
Updated•18 years ago
|
Target Milestone: --- → Firefox 3 beta1
Updated•17 years ago
|
Target Milestone: Firefox 3 M7 → Firefox 3 M8
Updated•17 years ago
|
Target Milestone: Firefox 3 M8 → Firefox 3 M9
Assignee | ||
Comment 12•17 years ago
|
||
Comment on attachment 265837 [details] [diff] [review]
always restore at least one non-popup window (and all popups closed afterwards)
Canceling review request as it's still not clear this is really the way to go and I might not have the time to implement it in a different way.
Attachment #265837 -
Flags: review?(dietrich)
Comment 13•17 years ago
|
||
We need to figure this out, but doesn't need to happen for b1
Target Milestone: Firefox 3 M9 → Firefox 3 M10
Updated•17 years ago
|
Target Milestone: Firefox 3 M10 → Firefox 3 M11
Comment 14•17 years ago
|
||
So what should be done here?
Is the solution suggested in comment 4 the way to go? In this case the patch probably needs to be updated.
Or is the solution suggested by the summary (ignore popup windows) prefered? Btw this would fix bug 393466.
Updated•17 years ago
|
Priority: -- → P5
Updated•17 years ago
|
Flags: wanted-firefox3+
Flags: blocking-firefox3-
Flags: blocking-firefox3+
Target Milestone: Firefox 3 Mx → Firefox 3 M11
Comment 16•17 years ago
|
||
From Bug 410883
"Session Restore seems to only work when all of FireFox is completely closed
down and then restarted, i need it to either allow a session restore
immediately or to remember some short session history that i may choose when
restarted so that i may recover my working session(s) from accidental mishaps."
Comment 17•17 years ago
|
||
Some of the proposals here seem to overlap with bug 409492.
Updated•16 years ago
|
Target Milestone: Firefox 3 beta3 → ---
Comment 20•16 years ago
|
||
I don't understand the behaviour of patch proposed in comment 4
Anyway, I think if this bug will be fixed, I hope this behaviour will be optional and disabled by default. Not all popups are evil and useless, and if Firefox closed for a crash or a blackout, you can lose some data.
Assignee | ||
Comment 21•16 years ago
|
||
(In reply to comment #20)
> Anyway, I think if this bug will be fixed, I hope this behaviour will be
> optional and disabled by default.
If we can help it there should be no optional behavior here - either we do it right or not at all. Then again, have you read the attachment's description ("always restore at least one non-popup window (_and_ all popups closed afterwards)")?.
Comment 22•16 years ago
|
||
(In reply to comment #21)
> have you read the attachment's description?
Of course, but I don't understand what it means.
Assignee | ||
Comment 23•16 years ago
|
||
Attachment #265837 -
Attachment is obsolete: true
Assignee | ||
Updated•16 years ago
|
Attachment #331774 -
Flags: ui-review?(beltzner)
Attachment #331774 -
Flags: review?(gavin.sharp)
Assignee | ||
Updated•16 years ago
|
Attachment #331774 -
Flags: ui-review?(beltzner) → review?(mano)
Assignee | ||
Updated•16 years ago
|
Attachment #331774 -
Flags: review?(gavin.sharp)
Comment 24•16 years ago
|
||
This is definitely a dataloss bug. It's really hard in such cases to restore the set of tabs which were opened, before accidentally closing the main window. I also ran into this trouble several times now and it's frustrating every time. We should push this forward.
Severity: normal → critical
Flags: blocking-firefox3.1?
Keywords: dataloss
Version: unspecified → Trunk
Assignee | ||
Updated•16 years ago
|
Comment 25•16 years ago
|
||
Simon, due to the dataloss we have here I believe we should let it stay at critical.
"Critical crashes, loss of data, severe memory leak"
Assignee | ||
Updated•16 years ago
|
Attachment #331774 -
Flags: review?(dietrich)
Comment 26•16 years ago
|
||
According to the severity description setting back the severity to critical:
https://bugzilla.mozilla.org/page.cgi?id=fields.html#bug_severity
Severity: major → critical
Comment 27•16 years ago
|
||
Comment on attachment 331774 [details] [diff] [review]
unbitrotted: always restore at least one non-popup window (and all popups closed afterwards)
r=me, thanks
Attachment #331774 -
Flags: review?(dietrich) → review+
Assignee | ||
Updated•16 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•16 years ago
|
Attachment #331774 -
Flags: review?(mano)
Comment 28•16 years ago
|
||
Status: NEW → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.1b1
Comment 29•16 years ago
|
||
verified with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20081003 Minefield/3.1b1pre
Status: RESOLVED → VERIFIED
Updated•16 years ago
|
Flags: blocking-firefox3.1?
Updated•16 years ago
|
Flags: in-litmus?
Comment 30•15 years ago
|
||
What is the expected behaviour with this bug? Do we now discriminate against pop-ups vs multiple open windows (i.e. multiple open windows will be restored but "pop-ups" will not)?
Please clarify for the purposes of writing a Litmus test case.
Comment 31•15 years ago
|
||
(In reply to comment #30)
> Please clarify for the purposes of writing a Litmus test case.
See the attachment description in comment 23.
Simon, can this be automated? If yes, we would still need a Litmus test for testing a real restart?
Flags: in-testsuite?
Assignee | ||
Comment 32•15 years ago
|
||
(In reply to comment #30)
> What is the expected behaviour with this bug?
That closing popups after the last non-popup browser window doesn't lose you that non-popup window (instead it and all popups closed afterwards should be restored). The attached testcase should help for creating a Litmus test.
Assignee | ||
Comment 33•15 years ago
|
||
(In reply to comment #31)
> can this be automated?
There's no way for automating crashes and restarts at the moment. Sorry.
Comment 34•15 years ago
|
||
(In reply to comment #33)
> (In reply to comment #31)
> > can this be automated?
>
> There's no way for automating crashes and restarts at the moment. Sorry.
Thanks Simon. Just one more question. Do you really need a crash for unit tests? Independently from the answer we can take care of it with a Mozmill test.
Assignee | ||
Comment 35•15 years ago
|
||
(In reply to comment #34)
> Do you really need a crash for unit tests?
No, a controlled restart works for almost all of the Session Restore functionality as well, especially for verifying that everything that should be restored actually is.
Comment 36•15 years ago
|
||
Simon, in your test case you note that it does not work on OSX because the X button does not quit Firefox. Would closing all windows with the X button then clicking Firefox menu > Quit satisfy the test for OSX?
Comment 37•15 years ago
|
||
I tested comment 36 on OSX...Minefield opens with a single window open to untitled using the following STR:
1. Open the Preferences dialog
2. Click the Content panel and uncheck "Block pop-up windows"
3. Click the Main panel and set "When Firefox starts" to "Show my windows and tabs from last time"
4. Close the Preferences dialog
5. Open this URL: <https://litmus.mozilla.org/testcase_files/firefox/5918/index.html>
6. Click the HOME button on the main window
7. Close the main window using the X button once your homepage loads
8. Close all pop-ups using the X button
9. Click Firefox menu > Quit
10. Start Firefox
Is this expected?
FYI, on Win/Lin, main window and pop-ups are restored (omitting step 9 of course).
Comment 38•15 years ago
|
||
in-litmus+
https://litmus.mozilla.org/show_test.cgi?id=7806
This is a preliminary test case. I'll update it as I get feedback on my previous comments.
Flags: in-litmus? → in-litmus+
Comment 39•15 years ago
|
||
(In reply to comment #37)
> I tested comment 36 on OSX...Minefield opens with a single window open to
> untitled using the following STR:
...
> Is this expected?
>
> FYI, on Win/Lin, main window and pop-ups are restored (omitting step 9 of
> course).
Yes, this is expected. See bug 481090.
Assignee | ||
Comment 40•15 years ago
|
||
(In reply to comment #38)
> https://litmus.mozilla.org/show_test.cgi?id=7806
Looks good, thanks.
You need to log in
before you can comment on or make changes to this bug.
Description
•