Closed Bug 1285587 Opened 3 years ago Closed 3 years ago

Cookies set by thumbnails appear for a short time before they're cleared

Categories

(Core :: DOM: Security, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: jhao, Assigned: jhao)

References

(Blocks 1 open bug)

Details

(Whiteboard: [userContextId][domsecurity-backlog][OA])

Attachments

(1 file, 2 obsolete files)

In bug 1279568, we use a private container for thumbnails so that the cookies wouldn't be set in the default container. But those cookies can be seen by users for a short period of time before the thumbnail finish loading and cookies are cleared.
We should also decide if we want to show these cookies in the cookie manager or we can hide them.
Probably it's better to hide them for several reasons:

1. they are stored without a real user navigation, but they are generated by internal navigations.
2. if privacy.userContext.enabled is disabled, the UI is confusing because it can be that, for the same domain, we have the same cookie shown twice (note that when containers are disabled by pref, we don't show the container name).
3. they will be removed automagically after a while.

tanvi?
Flags: needinfo?(tanvi)
(In reply to Andrea Marchesini (:baku) from comment #1)
> We should also decide if we want to show these cookies in the cookie manager
> or we can hide them.
> Probably it's better to hide them for several reasons:
> 
> 1. they are stored without a real user navigation, but they are generated by
> internal navigations.
> 2. if privacy.userContext.enabled is disabled, the UI is confusing because
> it can be that, for the same domain, we have the same cookie shown twice
> (note that when containers are disabled by pref, we don't show the container
> name).
> 3. they will be removed automagically after a while.
> 
> tanvi?


Do we use the temporary cookie jar for thumbnails when containers is disabled?

How long do they temporarily exist?  Will they even exist long enough for a user to find them in the cookie manager?

On one hand, I agree that we should not show them.  But something makes me uneasy about that.  I guess private mode cookies are not shown anywhere since they are temporary too.  But private mode cookies are in memory, and these are on disk.
Flags: needinfo?(tanvi)
Priority: -- → P1
Whiteboard: [userContextId][domsecurity-backlog] → [userContextId][domsecurity-backlog][OA]
Jonathan answered the above questions in our call (Jonathan can add more comments about that here).  We decided they should be hidden from the cookie manager.
(In reply to Tanvi Vyas - behind on reviews [:tanvi] from comment #2)
> Do we use the temporary cookie jar for thumbnails when containers is
> disabled?

Yes

> How long do they temporarily exist?  Will they even exist long enough for a
> user to find them in the cookie manager?

If a user opens the cookie manager before she opens a newtab and triggers the load, the temporary cookies will appear. In no more that a couple of seconds, the cookies will be deleted, but they won't disappear from the cookie manager because cookie manager doesn't refresh the view when it gets "cookie-changed" and "deleted", according to http://searchfox.org/mozilla-central/source/browser/components/preferences/cookies.js#111.
Assignee: nobody → jhao
Status: NEW → ASSIGNED
Paolo, please review this one. Thanks.
Attachment #8773662 - Flags: review?(paolo.mozmail)
Even if the code is in the front-end it supports new logic I'm not familiar with. If you're not in a hurry, I may be able to review this later next week, or you could ask Jared Wein or even Josh Matthews for the logic changes.
Comment on attachment 8773662 [details] [diff] [review]
Cookies set by thumbnails shouldn't appear.

Hi Josh, could you review this patch? Thanks.
Attachment #8773662 - Flags: review?(josh)
Comment on attachment 8773662 [details] [diff] [review]
Cookies set by thumbnails shouldn't appear.

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

Looks good!
Attachment #8773662 - Flags: review?(josh) → review+
Attachment #8773662 - Attachment is obsolete: true
Attachment #8773662 - Flags: review?(paolo.mozmail)
Attachment #8774995 - Flags: review+
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/23acad9c31c7
Cookies set by thumbnails shouldn't appear. r=jdm
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/23acad9c31c7
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.