419596 - add third party cookie blocking option to cookie pref UI

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect, P1)

Description

[[e-mail]]

per bug 417800 comment 7 and bug 417800 comment 11, it'd be nice to add this feature back. it's actually quite effective at blocking unwanted cookies (including from iframes and redirects within a page); in fact, it can be too effective since it breaks some sites.

while it's never fun to have an option that breaks things, i think there are two good arguments for it:

1) users who do want the additional strictness will feel better having the option there, and this reflects positively on our product - given that IE and Safari have third party blocking by default (though their implementations are quite toothless);

2) users who care enough to set it can probably also figure out how to use whitelisting for sites they need to work. (it would be super awesome to give the whitelisting feature some UX love as a followup to this, so that it's more discoverable and usable.)

Comment 1

[[e-mail]]

-> me for b5

Comment 2

[[e-mail]]

Attachment: patch v1

alright! this implements UI like so:
[x] Accept cookies from sites
    [x] for the originating site only

most of this is copy/pasted from firefox 1.5 code, when we still had this option in the prefpanel. (mapping a tristate pref to two checkboxes is a bit hackish - see _acceptOriginatingWasChecked in the js - but is probably best UI-wise.) i like the wording we used then, so i kept it.

i went with a simple indented checkbox, rather than the indented vbox container we used in 1.5. i also left the "Show cookies..."  button in the "Keep until:" hbox (ie on the third line).

firefox 1.5 reference:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/locales/en-US/chrome/browser/preferences/privacy.dtd&rev=1.9&cvsroot=/cvsroot
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/components/preferences/privacy.xul&rev=1.11&cvsroot=/cvsroot&mark=221-245#206
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla%2Fbrowser%2Fcomponents%2Fpreferences%2Fprivacy.js&rev=1.11&cvsroot=%2Fcvsroot&mark=101-135#101

i'm going to split out the string change in a separate patch, to see about landing it separately for the upcoming freeze.

Comment 3

[[e-mail]]

Attachment: patch v1, string only

just the string change. (if i need ui-r or somesuch here too, please speak up!)

Comment 4

[[e-mail]]

Attachment: prefpanel screenshot

Comment 5

[[e-mail]]

by way of testing: i've verified the pref is twiddled appropriately for the various combinations, that things get disabled and enabled properly, and that _acceptOriginatingWasChecked correctly remembers the checkbox state while the panel's open.

Comment 6

[Johnathan Nightingale [:johnath]]

(In reply to comment #3)
> Created an attachment (id=307680) [details]
> patch v1, string only
> 
> just the string change. (if i need ui-r or somesuch here too, please speak up!)

In principle I can ui-r sec-related stuff, but this is something that beltzner has been rotating on a great deal, and yes, introducing the strings will require ui-r (and explicit patch approval).  I know that theoretically he could just ui-r the final patch, but then if he wanted changes made, we'd be post-freeze.

Comment 7

[[e-mail]]

Comment on attachment 307680 [details] [diff] [review]
patch v1, string only

ok - thanks for the heads up!

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

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

I'm not sure we want to keep the sentence structure thing, and to get around the fact that we don't actually actively block third-party cookies, I'd suggest that the actual UI look like this:

[x] Accept cookies from sites
    [x] Accept third-party cookies

This means a string change to your patch, as well as an inversion of the UI such that when checked, the value of network.cookie.cookieBehavior is 0, and when unchecked it's 1.

(If you make it do that, you don't need a follow-on ui-r)

Comment 9

[[e-mail]]

Attachment: patch v2

as requested. (note, the checkbox will now default to true.)

Comment 10

[[e-mail]]

Attachment: patch v2, string change (checked in)

string change for freeze.

Comment 11

[[e-mail]]

see also bug 421494 for more better.

Comment 12

[[e-mail]]

Comment on attachment 307855 [details] [diff] [review]
patch v2, string change (checked in)

requesting approval on just the string change, for landing tonight.

Comment 13

[Johnathan Nightingale [:johnath]]

Comment on attachment 307855 [details] [diff] [review]
patch v2, string change (checked in)

>Index: browser/locales/en-US/chrome/browser/preferences/privacy.dtd
>+<!ENTITY  acceptThirdParty.label        "Accept third-party cookies">
>+<!ENTITY  acceptThirdParty.accesskey    "g">
>+

'g' is not a very intuitive access key for "Accept third-party cookies" - are all the other letters taken, or is this hold-over from the old string?

Comment 14

[[e-mail]]

(In reply to comment #13)
> 'g' is not a very intuitive access key for "Accept third-party cookies" - are
> all the other letters taken, or is this hold-over from the old string?

a holdover - indeed, with the new string, 'p' seems better, i'll do that on checkin. ('t' is unfortunately taken!)

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 307855 [details] [diff] [review]
patch v2, string change (checked in)

a1.9=beltzner

Comment 16

[[e-mail]]

Comment on attachment 307855 [details] [diff] [review]
patch v2, string change (checked in)

landed, with an accesskey of "p".

Comment 17

[Steffen Wilberg]

Firefox 1.5 documentation is at http://mxr.mozilla.org/mozilla1.8.0/source/browser/locales/en-US/chrome/help/cookies.xhtml#93

Comment 18

[[e-mail]]

Attachment: patch v3

updated per discussion with gavin - drop the variable to store 'third party' checkbox state, and just check it whenever 'accept cookies' is checked.

Comment 19

[[e-mail]]

Checking in browser/components/preferences/privacy.js;
/cvsroot/mozilla/browser/components/preferences/privacy.js,v  <--  privacy.js
new revision: 1.26; previous revision: 1.25
done
Checking in browser/components/preferences/privacy.xul;
/cvsroot/mozilla/browser/components/preferences/privacy.xul,v  <--  privacy.xul
new revision: 1.27; previous revision: 1.26
done

Comment 20

[Chris Ilias [:cilias]]

Thanks for the link Steffen. :-)

Comment 21

[(no longer active)]

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050810 Minefield/3.0pre

Verified.  Other platforms need verification as well.

Comment 22

[Chris Blore]

VERIFIED Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9) Gecko/2008051202 Firefox/3.0