Closed
Bug 364972
Opened 18 years ago
Closed 18 years ago
[SessionStore] allow SessionStore to work without writing data to disk
Categories
(Firefox :: Session Restore, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: zeniko, Assigned: zeniko)
Details
(Keywords: privacy, verified1.8.1.2)
Attachments
(1 file, 1 obsolete file)
5.77 KB,
patch
|
dietrich
:
review+
jay
:
approval1.8.1.2+
|
Details | Diff | Splinter Review |
sessionstore.js should only be written when either (1) resume_from_crash is true or (2) we need the data for session resuming.
This allows privacy concerned people to disable crash recovering while keeping full Undo Close Tab functionality, i.e. the recommended way to disable Session Restore would become setting sessionstore.resume_from_crash to |false| and _not_ setting sessionstore.enabled to |false| (which really is only for extensions providing the same functionality in a different way).
Assignee | ||
Comment 1•18 years ago
|
||
Assignee | ||
Comment 2•18 years ago
|
||
Branch drivers: fixing this bug on the branch would help those users who don't want crash recovery (for privacy reasons) but still want to be able to reopen accidentally closed tabs.
Flags: blocking1.8.1.2?
Updated•18 years ago
|
Flags: blocking1.8.1.2? → blocking1.8.1.2+
Updated•18 years ago
|
Attachment #249625 -
Flags: review?(dietrich)
Comment 3•18 years ago
|
||
Comment on attachment 249625 [details] [diff] [review]
make writing to disk dependent on resume_from_crash
This works ok, assuming default prefs. However, after applying this patch, if browser.startup.page=3 and resume_from_crash=false, sessions are not restored and this error is in the console:
Error: this._sessionFile has no properties
Source File: file:///Users/dayala/moz/branch/build/dist/BonEcho.app/Contents/MacOS/components/nsSessionStore.js
Line: 1720
Attachment #249625 -
Flags: review?(dietrich) → review-
Assignee | ||
Comment 4•18 years ago
|
||
Yeah, of course we need a reference to the session files first before trying to remove the referenced files... sorry for that.
Attachment #249625 -
Attachment is obsolete: true
Attachment #250222 -
Flags: review?(dietrich)
Attachment #249625 -
Flags: review?(gavin.sharp)
Comment 5•18 years ago
|
||
Comment on attachment 250222 [details] [diff] [review]
fix v2
thanks for fixing that, r+.
Attachment #250222 -
Flags: review?(dietrich) → review+
Assignee | ||
Updated•18 years ago
|
Whiteboard: [checkin needed]
Assignee | ||
Comment 6•18 years ago
|
||
Gavin: may I ask you for another checkin? Thanks.
And while you're at it: bug 340895 could also need some CVS love... ;-)
Comment 7•18 years ago
|
||
(In reply to comment #6)
> Gavin: may I ask you for another checkin? Thanks.
>
> And while you're at it: bug 340895 could also need some CVS love... ;-)
>
i checked this in:
Checking in browser/components/sessionstore/src/nsSessionStore.js;
/cvsroot/mozilla/browser/components/sessionstore/src/nsSessionStore.js,v <-- nsSessionStore.js
new revision: 1.56; previous revision: 1.55
done
i'll do 340895 as well.
Assignee | ||
Updated•18 years ago
|
Whiteboard: [checkin needed] → [baking]
Assignee | ||
Updated•18 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 8•18 years ago
|
||
Comment on attachment 250222 [details] [diff] [review]
fix v2
Drivers: See comment #2 for why this low-risk patch should be included on the branch.
Attachment #250222 -
Flags: approval1.8.1.2?
Comment 9•18 years ago
|
||
Comment on attachment 250222 [details] [diff] [review]
fix v2
Approved for the 1.8 branch, a=jay for drivers. Let's get this landed asap and get QA some time to test it thoroughly.
Attachment #250222 -
Flags: approval1.8.1.2? → approval1.8.1.2+
Assignee | ||
Comment 10•18 years ago
|
||
Dietrich: the patch should directly apply to the branch as well. Could you please check it in and mark this bug fixed1.8.1.2? Thanks.
Whiteboard: [baking] → [checkin needed (1.8 branch)]
Comment 11•18 years ago
|
||
Checking in browser/components/sessionstore/src/nsSessionStore.js;
/cvsroot/mozilla/browser/components/sessionstore/src/nsSessionStore.js,v <-- nsSessionStore.js
new revision: 1.5.2.42; previous revision: 1.5.2.41
done
Keywords: fixed1.8.1.2
Whiteboard: [checkin needed (1.8 branch)]
Comment 12•18 years ago
|
||
Please add test steps so QA can regress. thanks.
Whiteboard: [need testcase]
Assignee | ||
Comment 13•18 years ago
|
||
Steps to reproduce:
1. Set the hidden pref browser.sessionstore.resume_from_crash to false
2. Make sure that there's no file sessionstore.js in your profile folder
3. Set Firefox to load the previously opened windows and tabs at startup
4. Close Firefox
5. Verify that sessionstore.js exists
6. Restart Firefox
7. sessionstore.js should be gone again
8. Set the pref from step 1 to true again
9. sessionstore.js exists once more
Updated•18 years ago
|
Component: General → Session Restore
Updated•18 years ago
|
QA Contact: general → session.restore
Comment 14•18 years ago
|
||
Verified using steps in comment #13: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2pre) Gecko/20070206 BonEcho/2.0.0.2pre
Also tried on Mac and Linux (ubuntu).
Keywords: fixed1.8.1.2 → verified1.8.1.2
Whiteboard: [need testcase]
You need to log in
before you can comment on or make changes to this bug.
Description
•