230350 - navigator.cookieEnabled does not look at site specific enabling/disabling of cookies

Status: RESOLVED DUPLICATE of bug 578534

(Core :: DOM: Core & HTML, defect)

Description

[George G. Bolgar]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20031007
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20031007

navigator.cookieEnabled is the standard way of determining whether the navigator
will accept cookies.  The trouble is that Mozilla only tells you if cookies are
disabled at a system wide level.  This is useless, since what the script wants
to know is if it can set cookies, not whether some other domain might be able to
set cookies.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[George G. Bolgar]

Attachment: HTML file to show problem

1. Put HTML of web server
2. Set globel cookies (in preferences) to enabled on a site by site basis.
3. Ensure that site has cookies enabled (in cookie manager).
4. Load HTML in browser - it will show cookies enabled.
5. Change site (in cookie manager) to reject cookies.
6. Reload HTML - it still says cookies are enabled (even though they should
have now been disabled for the site).
7. Go into preferences, and disbale all cookies.
8. Reload HTML - it now, correctly, says that cookies are disabled.

Comment 2

[Fabian Guisset]

Confirmed, marking NEW. Sending to DOM Level 0. Probably just a matter of
reusing what already exists.

Comment 3

[matt c]

I just (re)discovered this bug, and can confirm that it (still) exists in Firefox 2.0 and Camino 1.0.4. 

It also occurs in reverse:

1. disable cookies globally
2. add a site in cookie manager to accept cookies 
3. verify javascript served from that site has document.cookieEnabled set to 'false' even though cookies are enabled for that site.

It's important to note that this bug basically makes document.cookieEnabled useless for detecting whether the user has cookies enabled or not.  IMHO, this bug should be higher priority.

Comment 4

[tiberius]

bug still exists in Firefox 2.0.0.11. i agree with matt c that this should be higher priority

Comment 5

[u166595]

(In reply to comment #4)
> bug still exists in Firefox 2.0.0.11. i agree with matt c that this should be
> higher priority

And in SeaMonkey 1.1.9
And in Firefox 2.0.0.13
(And - as an aside - in Opera...)
In at least one case this bug blocks logging in for online banking.
So indeed, it really should be higher - if not high - priority.

Comment 6

[Daniel.S]

Logging in with Microsoft's Live ID or whatever it actually is seems to be blocked by this as well.
Currently, I'm not able to visit the Connect Bug Tracking System of Microsoft (where Live ID is used) when my cookies are disabled. That's why I visit the site in my cookie enabled Opera for now.

Comment 7

[:aceman]

I see it on linux too, is probably platform independent.

Comment 8

[Johnny Stenback (:jst)]

Not a wanted 1.9.1 bug, but that doesn't mean we wouldn't accept a patch if someone works one up.

Comment 9

[Daniel.S]

I figured that the code at http://mxr.mozilla.org/mozilla-central/source/dom/src/base/nsGlobalWindow.cpp#9136 only looks at the global preference.

Can anyone give me a hint, how I can get the site specific preference at that point? The rest would be trivial.

Comment 10

[Dão Gottwald [:dao]]

not a polish bug

Comment 11

[u278084]

Attachment: v1

I guess I can take this bug. It seems easy to fix, and it sorta annoys me.

Here is a first stab at it. I appreciate a quick drive-by review, but I'm not sure who the right person is. jst would you take a look at it?

Tests will come soon, and that will come when I ask for actual review.

Comment 12

[u278084]

Attachment: v2 with test

Here is a patch with tests.

Comment 13

[Daniel.S]

ping.

Sorry. Dan seems to be very busy, is there anyone else who could review this?

Comment 14

[[e-mail]]

Comment on attachment 362326 [details] [diff] [review]
v2 with test

Hmm. Seems like a good idea, but I worry that we're exposing more information by doing this. For instance, it'd be relatively simple for a site to figure out if you have an exception in place for it. Is that acceptable? jst or dveditz should probably make a call there.

Also, the value of navigator.cookieEnabled has no real bearing on whether the site will be able to set or retrieve cookies, with or without this patch. There are many other things that can get in the way (user prompting, third party context, privacy addons etc). But this does get us closer to the intent of navigator.cookieEnabled, which is a good thing.

On to the review...

> NS_IMETHODIMP
> nsNavigator::GetCookieEnabled(PRBool *aCookieEnabled)
> {
>-  *aCookieEnabled =
>-    (nsContentUtils::GetIntPref("network.cookie.cookieBehavior",
>-                                COOKIE_BEHAVIOR_REJECT) !=
>-     COOKIE_BEHAVIOR_REJECT);
>+  nsCOMPtr<nsIChannel> channel;
>+  nsCOMPtr<nsIURI> uri;
>+  nsCString string;

You don't use |string|, you can remove it.

>+  nsCOMPtr<nsICookiePermission> perm =
>+    do_GetService(NS_COOKIEPERMISSION_CONTRACTID);
>+  nsCookieAccess access;
>+
>+  if (!perm) {
>+    return NS_ERROR_NOT_IMPLEMENTED;
>+  }

Not having an nsICookiePermission service isn't fatal. You should fall back to the previous behavior in that case...

>+  if (NS_SUCCEEDED(mDocShell->GetCurrentDocumentChannel(
>+        getter_AddRefs(channel))) &&
>+      NS_SUCCEEDED(channel->GetURI(getter_AddRefs(uri))) &&
>+      NS_SUCCEEDED(uri->GetHost(string)) &&

You don't need the GetHost() call here. Also, I'm not sure if you're getting the correct URI here (check with jst?). (For example, http://mxr.mozilla.org/mozilla-central/source/content/html/document/src/nsHTMLDocument.cpp#1732 involves getting the codebase URI.)

>+      NS_SUCCEEDED(perm->CanAccess(uri, channel, &access))) {
>+          switch (access) {
>+            case nsICookiePermission::ACCESS_DENY :
>+              /* We explicitly deny this domain */
>+              *aCookieEnabled = PR_FALSE;
>+              break;
>+            case nsICookiePermission::ACCESS_ALLOW:
>+              *aCookieEnabled = PR_TRUE;
>+              break;
>+            default:
>+              /* Check preferences for ACCESS_DEFAULT */
>+              *aCookieEnabled =
>+                (nsContentUtils::GetIntPref("network.cookie.cookieBehavior",
>+                  COOKIE_BEHAVIOR_REJECT) != COOKIE_BEHAVIOR_REJECT);

If the cookieBehavior pref doesn't exist, the cookieservice will actually default to accepting cookies... so the second arg to GetIntPref should probably be COOKIE_BEHAVIOR_ACCEPT, which you can #define to be 0 above.

Also, please indent the |switch| statement two spaces in from the |if|.

>+          }
>+  } else {
>+    return NS_ERROR_FAILURE;
>+  }

Could just return the default value instead of erroring.

Comment 16

[Daniel.S]

(In reply to comment #14)
> But this does get us closer to the intent of navigator.cookieEnabled,
> which is a good thing.

And I think that's what this bug is actually about.

I surf the web with cookies disabled myself. Unfortunately, there are a lot of websites that use cookieEnabled to check if they should show me their content or not.

So even if I allow example.org to set cookies, it won't let me log in or do any other action, because cookieEnabled returns the wrong value. That's really annoying.

Comment 17

[Alice0775 White]

is flag wanted1.9.2 ?

Comment 18

[bugreport]

Is there any chance of getting this fixed?  It was reported SIX YEARS ago and many versions have come and gone since.  Seems like a pretty important error.  If anything, count this as a request to bump up the priority level on this one.

Comment 19

[Aaron Slunt]

There hasn't been a patch in this bug for a year and a half, and we have another bug opened where there is progress being made. Marking this as a dupe of that. If I'm wrong for doing this, someone is more than welcome to correct my action.