810794 - Setting to no accept 3rd party cookies

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Firefox for Android cannot set to no accept 3rd party cookies.
(So current implementation can config "network.cookie.cookieBehavior" to 0 or 2.)

Comment 1

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Attachment: patch v1

Comment 2

[:Margaret Leibovic]

I think we should ask Ian for some UX feedback here. Right now this pref is in the middle of a big list of checkboxes, so it might be weird to have a tri-state pref in the middle.

+<!ENTITY pref_cookies "Cookies">
+<!ENTITY pref_cookies_accept_all "Accept all">
+<!ENTITY pref_cookies_not_accept_foreign "Not accept 3rd">
+<!ENTITY pref_cookies_disabled "Disabled">

I also think we need a better string than "Not accept 3rd". Maybe "Don't accept 3rd party" or "Disable 3rd party". I also think we should use "Enabled" instead of "Accept all" for consistency with "Disabled".

Comment 3

[:Margaret Leibovic]

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

The approach is good here, I just want to get UX input before making it an r+/

Comment 4

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Margaret, thank you for reviewing rapidly.

(In reply to Margaret Leibovic [:margaret] from comment #2)
> I also think we need a better string than "Not accept 3rd". Maybe "Don't
> accept 3rd party" or "Disable 3rd party". I also think we should use
> "Enabled" instead of "Accept all" for consistency with "Disabled".

I agree your point.
How do you think "Disabled all" instead of "Disabled"? This has a contrast with "Disabled 3rd party". But I don't conclude it is natural or not because I'm not native English speaker.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

We have two other settings that are similar: "Automatic updates" and "Plugins"

Both use "Enabled" and "Disabled", although the positioning in the list is different.

Comment 6

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

(In reply to Mark Finkle (:mfinkle) from comment #5)
> We have two other settings that are similar: "Automatic updates" and
> "Plugins"
> 
> Both use "Enabled" and "Disabled", although the positioning in the list is
> different.

(I don't know about details of their implemenetations,)
At list of "Plugins", I think that the positioning in it means a stage of preference. e.g. "Enabled (always)"->"Tap to play (when user activate)"->"Disabled (always)".
And also I use same approach in attachment 680565 [details] [diff] [review].

But I seem that it is strange which the list of "Automatic updates"... I think it should be better with changing to this order: "Enabled"->"Only over Wi-Fi"->"Disabled".

Comment 7

[Ian Barlow (:ibarlow)]

I'm fine to add this in. My suggested text would be:

+<!ENTITY pref_cookies "Cookies">
+<!ENTITY pref_cookies_accept_all "Enabled">
+<!ENTITY pref_cookies_not_accept_foreign "Enabled, excluding 3rd party">
+<!ENTITY pref_cookies_disabled "Disabled">


I am not hugely concerned with it being adjacent to checkbox list items, but I might rearrange things a bit, to keep more similar items together: 

Cookies 
Clear private data
Tell sites not to track me []
Remember passwords []
Use master password []
Show search suggestions []
Send performance data []
Show product announcements []


> But I seem that it is strange which the list of "Automatic updates"... I
> think it should be better with changing to this order: "Enabled"->"Only over
> Wi-Fi"->"Disabled".

I agree, that is a more logical ordering of choices.

Comment 8

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

(In reply to Ian Barlow (:ibarlow) from comment #7)
> I'm fine to add this in. My suggested text would be:
> 
> +<!ENTITY pref_cookies "Cookies">
> +<!ENTITY pref_cookies_accept_all "Enabled">
> +<!ENTITY pref_cookies_not_accept_foreign "Enabled, excluding 3rd party">
> +<!ENTITY pref_cookies_disabled "Disabled">

Thank you. I take them in a new patch.


> I am not hugely concerned with it being adjacent to checkbox list items, but
> I might rearrange things a bit, to keep more similar items together: 

I filed Bug 811285 and CCing you.


> > But I seem that it is strange which the list of "Automatic updates"... I
> > think it should be better with changing to this order: "Enabled"->"Only over
> > Wi-Fi"->"Disabled".
> 
> I agree, that is a more logical ordering of choices.

I filed Bug 811284.

Comment 9

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Attachment: patch v2

Update local strings (I take Ian's comment #7)

Comment 10

[:Margaret Leibovic]

Comment on attachment 681022 [details] [diff] [review]
patch v2

Great! Thanks for the patch!

Also, thank you for filing those follow-up bugs, as well.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f82641cc61eb

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f82641cc61eb

Comment 13

[Aaron Train [:aaronmt]]

Entity wasn't changed here despite a string change. See blocking bug.

Comment 14

[:Margaret Leibovic]

(In reply to Aaron Train [:aaronmt] from comment #13)
> Entity wasn't changed here despite a string change. See blocking bug.

Doh, that was a dumb mistake on my part. Sorry.

Comment 15

[Cristian Nicolae (:xti)]

The setting was added on the latest Nightly. Closing bug as verified fixed on:

Firefox 20.0a1 (2012-11-20)
Device: Galaxy S2
OS: Android 4.0.3

Comment 16

[Ioana Chiorean]

TCs added in Moztrap with new option for following versions: 19, 19 tablets, 20 , 20 tablets 

https://moztrap.mozilla.org/manage/case/5874 - options for Cookies
https://moztrap.mozilla.org/manage/case/5876- default value for Cookies