215460 - accept cookie dialog's "...all cookies from this site" checkbox does not always suppress further prompting

Status: VERIFIED FIXED

(Core :: Networking: Cookies, defect, P1)

Description

[Steve]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.5b) Gecko/20030804
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.5b) Gecko/20030804

A new cookie from a site causes the popup to come up.  The checkmark for
"remember for this site" IS checked.  Clicking on either YES or NO seems to have
no affect though, as further cookies being set or updated brings up the popup.

Reproducible: Always

Steps to Reproduce:
1.Cookie wants to be set or updated.
2.Popup to accept or deny comes up.
3.Selection is made but the next cookie from the same server causes the popup to
comeup again and again.



Expected Results:  
If the checkmark is selected then the it should remember and not ask again.

Comment 1

[Stefan [:stefanh]]

Steve, can you please provide some more info? Maybe provide an URL with some
steps to reproduce the problem?

Comment 2

[Steve]

Attachment: First Cookie confirmation box.

Notice that it says I already have 6 cookies from this site and it is asking to
set another.

Comment 3

[Steve]

Attachment: Second cookie conformation grab.

This is the second dialog I got just seconds afte rthe first.  Notice it says
that I have 7 cookies from this site.

Comment 4

[Steve]

it's not consistent with every site I visist that has cookies but I have noticed
it mainly when I get a third party cookie.  I have added two attachments to show
what happens when I go to the hotmail.com page.

Comment 5

[[e-mail]]

mvl, wanna look at this?

maybe something amiss in the domain-matching fu, or maybe the UI is broken? :)

Comment 6

[Michiel van Leeuwen (email: mvl+moz@)]

Steve, is the host you are visiting (and appears in the cookie dialog) in the
list of sites in the cookie manager?
What are your cookie preferences?
Does this also happens with a clean profile?

Comment 7

[Steve]

The hotmail.com domain is in the cookie manager, but some are not like a cookie
I just set to not accept, from specificlick.com, it is not in the cookie manager.

My cookie prefs are:  Accept all cookies,Ask me before storing a cookie.

I tried a clean profile and it still happens.

Comment 8

[Chris Casciano]

My scenario with the same settings (OS X Moz & Camino trunk) -

- Visit site A (say: http://www.sierratradingpost.com/)
- First page load* sets 3 or 4 cookies for site A and one for ad site B
- I respond to the first prompt for site A cookies with "Allow" "Use my choice
for all..." however I still get prompted to react to the next 2 or 3 cookies for
site A before I see the cookie for site B
- Visiting further pages on site A has *not* triggered a prompt I didn't expect
leading me to believe that my actions took

So in my experience it behaves as if the prompts are getting queued up for the
first page view and they're just not being handled intelligently (that or the
pref isn't written until all the action are done).

Steve, does this jive with what you are seeing? Are you being prompted for
cookies for the site beyond the first page load, or is it simply multiple
cookies on the first page load that aren't behaving as you expect?



* I haven't yet looked to see if all the cookies are coming from the page, some
from the page some from images or what...

Comment 9

[Steve]

At first glace it would appear that yes they are being queued, but I'd say 80%
of the time if I go to a different section of a site or do a reload, I would get
swamped with confirmations to set cookies that I already made a decision about.

At the <http://www.sierratradingpost.com/> site I get 3 confirmations to set a
cookie for teh same server.  I click Allow and "Use my choice..." is ckecked as
fast aas they come up.  A few seconds later a 3rd party cookie confirmation pops
up.  Pre Build 20030807. I would have just got 2 confirmation boxes, one for the
first server and one for the second.  If a site sets a lot of cookies or even
modifies a lot, I have to keep cliking to allow the page to load

Comment 10

[Michiel van Leeuwen (email: mvl+moz@)]

dwitte, could this be a regression from bug 184059?
That made it so that first the permmanager is asked, and the SetCookie: headed
is parsed after that. If there are multiple cookies in that header, all of them
will use the same cached permission. If that is "unknown", and the pref is to
prompt, there will be a prompt for all the cookies, even if there is a new perm
stored (by using the checkbox), due to the caching.

I don't know if that explains all the comments in this bug, but this should be
fixed before we can look at the rest :)

Comment 11

[Darin Fisher]

yeah, i think mvl hit the nail on the head.  the patch for bug 184059 needs to
be revised or backed out for 1.5 beta as i don't think it does much good at the
moment without any UI to configure cookie permission white-lists.

Comment 12

[benc]

"Cookie preferences are not saved" was too vague...

Comment 13

[[e-mail]]

sorry for the delay guys, back from vacation now. :)

so... would it not be preferable to just fix the regression? i can whip up a
patch tonight if desired. darin, if you'd rather back out, fine by me; i'm not
up-to-date with how beta is going, so...

Comment 14

[Darin Fisher]

Attachment: v1 patch

dwitte convinced me to try fixing this bug directly instead of backing out the
patch which introduced this problem.  so, here it is.  i setup a local
testcase, and this patch seems to solve the problem.

Comment 15

[[e-mail]]

Comment on attachment 129752 [details] [diff] [review]
v1 patch

looks good, thanks!

>Index: nsCookiePermission.cpp
>===================================================================
> NS_IMETHODIMP 
> nsCookiePermission::TestPermission(nsIURI *aURI,
>                                    nsICookie *aCookie,
>                                    nsIDOMWindow *aParent,
>                                    PRInt32 aCookiesFromHost,
>                                    PRBool aChangingCookie,
>                                    PRBool aShowDialog,
>-                                   PRUint32 aListPermission,
>                                    PRBool *aPermission) 
...
>+  nsresult rv;
>+  nsCOMPtr<nsIPermissionManager> permMgr =
>+      do_GetService(NS_PERMISSIONMANAGER_CONTRACTID, &rv);
>+  if (NS_FAILED(rv)) return rv;

since we're doing this every time, can we cache the permmgr? either something
simple like |if (!mPermMgr) { do_GetService }|, or we could go and add the
::Init method back again, but that'd be way overkill :/

r=me regardless.

Comment 16

[Darin Fisher]

Attachment: v1.1 patch: revised per comments from dwitte

ok, added code to cache the nsIPermissionManager reference.

Comment 17

[Darin Fisher]

Comment on attachment 129756 [details] [diff] [review]
v1.1 patch: revised per comments from dwitte

carrying over r=dwitte...

bryner: the major change here is that we need to call
nsIPermissionManager::TestPermission each time before putting up the "accept
this cookie" dialog.  this is necessary because a single call to
nsICookieService::SetCookieString could actually result in several cookies
being set.

Comment 18

[Asa Dotzler [:asa]]

This will need to happen by the end of the week if it's gonna make the 1.5b train.

Comment 19

[Asa Dotzler [:asa]]

Comment on attachment 129756 [details] [diff] [review]
v1.1 patch: revised per comments from dwitte

a=asa (on behalf of drivers) for checkin to 1.5beta

Comment 20

[Darin Fisher]

fixed-on-trunk

Comment 21

[Chris Casciano]

V. Fixed

Neither Camino 081810 nor Seamonkey 081703/OS X show any signs of this problem
anymore... get prompted once per domain as expected