Note: There are a few cases of duplicates in user autocompletion which are being worked on.

navigator.cookieEnabled does not take exceptions into account.

RESOLVED FIXED in mozilla8



DOM: Core & HTML
7 years ago
3 years ago


(Reporter: Alex, Assigned: Sindre Dammann)



Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)




(1 attachment, 3 obsolete attachments)



7 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20100407 Ubuntu/9.04 Shiretoko/3.5.9
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20100407 Ubuntu/9.04 Shiretoko/3.5.9

If a website tries to check whether it can set cookies by looking at navigator.cookieEnabled when cookies are disabled globally, it will return false even if there is an exception allowing the site. This can result in being redirected to a error page claiming cookies are required, when the site is actually able to set cookies perfectly fine.

Reproducible: Always

Steps to Reproduce:
1. Uncheck "Accept cookies from sites" in preferences.
2. Add an exception to allow an affected site.
3. Go to the site.
Actual Results:  
navigator.cookieEnabled will return false for the site even though it can set cookies.

Expected Results:  
navigator.cookieEnabled should return true for this site, because it is able to set cookies.

Comment 1

7 years ago
Confirming Mozilla/5.0 (X11; Linux x86_64; en-US; rv:2.0b2pre) Gecko/20100711 Gentoo Firefox/4.0b2pre
Ever confirmed: true

Comment 2

7 years ago
Created attachment 466171 [details] [diff] [review]
Proposed patch + test

I don't know who to request review from.
Attachment #466171 - Flags: review?


7 years ago
Attachment #466171 - Flags: review? → review?(dwitte)

Comment 3

7 years ago
Oh great a patch!
Maybe I would have confirmed and marked this bug a dupe of bug 230350 if I would have been allowed to do so.
If you follow that link you will probably learn something about 'review process' and 'patience' there...
Seriously: thank you Sindre, this bug is annoying as hell!


7 years ago
Duplicate of this bug: 230350

Comment 5

7 years ago
@Sindre: +1
Component: General → DOM: Core & HTML
Product: Firefox → Core
QA Contact: general → general
Version: unspecified → Trunk

Comment 6

7 years ago
Created attachment 467869 [details] [diff] [review]
Patch v2 + Test v2

This version uses nsCOMPtr every time, and resorts to the old behavior if any part of checking for an exception fails (The review in Bug 230350 makes it sound like this is desired). 

Also changed a bunch of other stuff so it hopefully conforms more with the Mozilla Coding style.
Attachment #466171 - Attachment is obsolete: true
Attachment #467869 - Flags: review?(dwitte)
Attachment #466171 - Flags: review?(dwitte)

Comment 7

7 years ago
Created attachment 470222 [details] [diff] [review]
Patch v3 + Test v3
Assignee: nobody → sindrebugzilla
Attachment #467869 - Attachment is obsolete: true
Attachment #470222 - Flags: review?(dwitte)
Attachment #467869 - Flags: review?(dwitte)

Comment 8

7 years ago
Created attachment 474447 [details] [diff] [review]
Patch v4 + Test v3

Uses do_QueryInterface instead of QueryInterface.
Attachment #470222 - Attachment is obsolete: true
Attachment #474447 - Flags: review?(dwitte)
Attachment #470222 - Flags: review?(dwitte)
Library catalogs based on Polaris Library Systems software are one example of a type of site known to be vulnerable to this problem.  (You can't even search the catalog if the cookie test fails -- it just tells you to enable cookies.  A per-site exception is useless, because the test still fails.)

For the same reason, if cookies are allowed globally but a site is blocked, the test will tell the site it can set cookies.  (This version of the issue is less likely to cause problems, but I thought I'd mention it anyway.)  I also created a testcase, here:
All this page does is test navigator.cookiesEnabled and report the result (via document.write).

Comment 10

6 years ago
Dan, can you (or anyone else) please have a look?
I know there are other important bugs, but this patch had its review requested half a year ago. Thank you. Sorry for bugspam.
Comment on attachment 474447 [details] [diff] [review]
Patch v4 + Test v3

Sid, can you review this patch or solicit a fresh one if this has rotted too much? Or pass r? along to a better person? Thanks,

Attachment #474447 - Flags: review?(dwitte) → review?(sstamm)

Comment 12

6 years ago
Comment on attachment 474447 [details] [diff] [review]
Patch v4 + Test v3

This looks great.  A few nits:

1)  Declare variables when you need them, not up front.
2)  Names starting with 'm' are for member variables.
3)  We should probably use the principal URI, just like we do in
4)  The channel is unused by the callee, and the only other caller passes null,
    so probably better to pass null.

I'll make those changes and get this checked in.  r=me
Attachment #474447 - Flags: review?(sstamm) → review+

Comment 13

6 years ago

Sindre, thank you for the patch!  I'm sorry that it took so long to get someone to look at it.  :(
Flags: in-testsuite+
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8


6 years ago
Duplicate of this bug: 495695

Comment 16

6 years ago
The documentation of document.cookieEnabled needs to be updated.


1. IE and Chrome also allow exceptions to the global cookie setting. However, IE has this issue as well, Chrome does not. Opera and Safari apparently do not allow exceptions.

2. Since cookieEnabled is not reliable, add a note that setting document.cookie and immediatly trying to read document.cookie is a more reliable method to check whether cookies can be set.

P.S. Thank you, Boris!
Keywords: dev-doc-needed
Docs updated:

And added to Firefox 8 for developers.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.