206356 - "Ask me dialog" should not "use my choice for all cookies" if dismissed

Status: RESOLVED FIXED

(Core :: Networking: Cookies, defect)

Description

[Adeyinka Joan]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4b) Gecko/20030516 Mozilla Firebird/0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4b) Gecko/20030516 Mozilla Firebird/0.6

If you close the cookie dialog without first clicking "Accept" or "Deny", all
cookies from the site are automatically denied.

Reproducible: Always

Steps to Reproduce:
1. Go to website that sends cookies.
2. When dialog pops up, close it.

Actual Results:  
Site is set as "cannot set cookies".

Expected Results:  
Closing the dialog window should not be allowed.

Comment 1

[[e-mail]]

confirmed on linux trunk... closing the dialog adds a permmgr entry to block the
site.

this doesn't quite seem logical... we should probably just not accept the cookie...

Comment 2

[[e-mail]]

clarification: the site is only added if 'remember my decision' is checked.

so 'close' operates identically to 'deny' in this case.

Comment 3

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

Attachment: only remember when using the buttons

This patch only remembers the decision when the buttons are used, not when the
dialog is closed. Closing the dialog still means that the cookie will be
rejected, but the perm list won't be touched anymore.

Comment 4

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

Comment on attachment 123779 [details] [diff] [review]
only remember when using the buttons

diff made a mess out of cookieAccept and cookieDeny.

So that is what the functions will look like:

function cookieAccept()
{
  // say that the cookie was accepted
  params.SetInt(nsICookieAcceptDialog.ACCEPT_COOKIE, 1); 
  // And remember that when needed
  params.SetInt(nsICookieAcceptDialog.REMEMBER_DECISION,
document.getElementById('persistDomainAcceptance').checked);
  window.close();
}

function cookieDeny()
{
  // say that the cookie was rejected
  params.SetInt(nsICookieAcceptDialog.ACCEPT_COOKIE, 0); 
  // And remember that when needed
  params.SetInt(nsICookieAcceptDialog.REMEMBER_DECISION,
document.getElementById('persistDomainAcceptance').checked);
  window.close();
}

Comment 5

[[e-mail]]

moa=dwitte for this, contingent on r=neil/sr=jag. reassigning to mvl.

Comment 6

[[e-mail]]

Comment on attachment 123779 [details] [diff] [review]
only remember when using the buttons

/me would have preferred cookieDeny first for a smaller patch :-P

Comment 7

[jag (Peter Annema)]

Comment on attachment 123779 [details] [diff] [review]
only remember when using the buttons

sr=jag. Do you think the accept and deny functions will always be similar
enough that it's worth moving their bodies out into another function that gets
called with 0 or 1?

Comment 8

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

Comment on attachment 123779 [details] [diff] [review]
only remember when using the buttons

I can't really put the functions in one. doSetOKCancel only takes functions,
you can't make it pass any parameters do the functions.
It would be possible to create two wrapper functions that call the unified
function, but i think that would be overkill for three lines of code.

Comment 9

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

Comment on attachment 123779 [details] [diff] [review]
only remember when using the buttons

requesting approval.
This makes the UI a bit more logical. It's low risk.

Comment 10

[Asa Dotzler [:asa]]

Comment on attachment 123779 [details] [diff] [review]
only remember when using the buttons

a=asa (on behalf of drivers) for checkin to 1.4.

Comment 11

[[e-mail]]

checked in to trunk. thx for the quick work!

Comment 12

[benc]

VERIFIED: Mach-O + Win 98 - 20030617 - commercial build.