Closed Bug 690992 Opened 8 years ago Closed 8 years ago

App tabs break deleting cookies on close (FF8+)


(Firefox :: Session Restore, defect)

8 Branch
Not set



Firefox 10
Tracking Status
firefox9 + fixed


(Reporter: cranberrydoughan, Assigned: zpao)



(Keywords: privacy, regression, Whiteboard: [fixed-in-fx-team][qa!])


(2 files)

User Agent: Mozilla/5.0 (X11; Linux i686; rv:8.0) Gecko/20100101 Firefox/8.0
Build ID: 20110929094716

Steps to reproduce:

I set cookies to be kept until I close Firefox (FF8 on Ubuntu 10.10). 
I went to websites with cookies.
With an app tab set, I closed Firefox.
I opened Firefox again.
I found that all cookies were still present.
I went to new websites with cookies.
I again closed Firefox.
I opened Firefox a third time. 
I found the cookies created in the very first session were gone, but those created in the second were still present.

Actual results:

The cookies created in the last session are still present. Cookies that were initially created in the session before the last are actually deleted.

This only seems to happen with an app tab open (content doesn't matter, it could even be blank). I also tested this with a new profile with no extensions, so that shouldn't matter. If I have no app tabs set, all cookies are properly deleted each time Firefox is closed. 

Expected results:

All cookies should be deleted every time Firefox is closed.
I just tested the newest beta build, this time on a fresh Ubuntu 11.10 install. The problem is still present.

I suppose that the cookies from app tabs might be supposed to be kept? Or are they just immediately recreated when you restart? Not sure, but regardless all the rest should be deleted upon exit. Firefox 7 and older versions do not have this problem.
Mozilla/5.0 (Windows NT 5.1; rv:10.0a1) Gecko/20111030 Firefox/10.0a1
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a1) Gecko/20111030 Firefox/10.0a1

Confirmed. The cookies from the previous session are still displayed after Firefox restart with an app tab. Another restart will erase them. 

Doesn't seem to occur for me on Firefox 7. To investigate for regression window.
Component: General → Session Restore
Ever confirmed: true
OS: Linux → All
QA Contact: general → session.restore
Hardware: x86 → All
Last good nightly: 2011-07-28
First bad nightly: 2011-07-29

I just found this independently while looking at code. Bug 668865 is the change that made this happen.

Aibara, I appreciate you filing this a month ago. I'm sorry for the delay in reading this (it got lost in the pile of vacation bugmail).

I may be fixing this for free thanks to bug 698565, so stay tuned.

If we want a quick aurora/beta fix, simply removing "_host" and "_scheme" from INTERNAL_KEYS at should fix the problem and be a safe fix.
Blocks: 668865
Keywords: privacy, regression
More specifically, this is happening because we get no hosts from _extractCookieHosts and so we end up building a regular expressions /.*()/ to match against hosts when splitting the app tab cookies out. every host is going to match that so :( We can build a more complete fix, but at this point I think the best short term action would be to back out bug 668865.
Attached patch Patch A+C v0.1Splinter Review
There were 3 options essentially.
A: simply include _host & _scheme in the data written to disk
B: backout bug 668865
C: add fallback in _extractHostsForCookie (if _host/_scheme don't exist, figure them out) & fix the regex check

A mostly works except for the 1 time upgrade (when quitting & starting, not through restart process). This doesn't fix the case where there is only 1 app tab & it's an about: url though (contrived, but not impossible)
B is ok
C is riskiest, but most complete.

This patch does A, but also takes the regex fix from C. This saves face for most cases and in the one time upgrade (quit & start), we will just throw away many cookies until the session is restored (essentially opposite behavior of this bug). This errs on the side of privacy. On future startups we'll be back to normal behavior.
Assignee: nobody → paul
Attachment #571459 - Flags: review?(dietrich)
Attached patch Patch C v0.1Splinter Review
Here is C. The tradeoff between these is data on disk vs xpcom. A+C would write more data to disk all the time. C will result in us generating (potentially a lot of) nsIURIs during 1 potential startup path (apptabs + not restoring)
Attachment #571499 - Flags: review?(dietrich)
Comment on attachment 571499 [details] [diff] [review]
Patch C v0.1

Review of attachment 571499 [details] [diff] [review]:

r=me, thanks for figuring this out!
Attachment #571499 - Flags: review?(dietrich) → review+
Attachment #571459 - Flags: review?(dietrich)
pushed to fx-team
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 10
Closed: 8 years ago
Resolution: --- → FIXED
Comment on attachment 571499 [details] [diff] [review]
Patch C v0.1

Since we're tracking this for Firefox 9, asking approval for aurora now.
Attachment #571499 - Flags: approval-mozilla-aurora?
Comment on attachment 571499 [details] [diff] [review]
Patch C v0.1

[triage comment]

Approved for aurora. Please land today.
Attachment #571499 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Mozilla/5.0 (X11; Linux x86_64; rv:9.0) Gecko/20100101 Firefox/9.0

Cookies received before selecting the "keep cookies until I close Firefox" option are still displayed after restarting. Is this expected?
(In reply to Virgil Dicu [QA] from comment #14)
> Mozilla/5.0 (X11; Linux x86_64; rv:9.0) Gecko/20100101 Firefox/9.0
> Cookies received before selecting the "keep cookies until I close Firefox"
> option are still displayed after restarting. Is this expected?

Yea, those prefs are in conflict. See bug 530594.
Whiteboard: [fixed-in-fx-team] → [fixed-in-fx-team][qa+]
Mozilla/5.0 (X11; Linux x86_64; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (Windows NT 6.1; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (Windows NT 5.1; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0) Gecko/20100101 Firefox/9.0

Verified in Firefox9Beta3, 10 and 11 on Mac OS 10.6, Windows XP, Windows 7 and Ubuntu 11.10.

Steps used:
1. Start Firefox with a clean profile. 
2. From Edit-Preferences-Privacy set "Use custom settings from history" and keep cookies until close Firefox. (remove all existing cookies)
3. Pin an app tab.
4. Visit any site which stores cookies (e.g. in a normal tab.
5. Check that there are cookies saved from that specific site.
6. Restart Firefox.

Upon restart all cookies are now deleted.
Whiteboard: [fixed-in-fx-team][qa+] → [fixed-in-fx-team][qa!]
You need to log in before you can comment on or make changes to this bug.