501910 - Consider Preferences UI for cookies

Status: RESOLVED FIXED

(Thunderbird :: Preferences, defect)

Description

[Mark Banner (:standard8)]

In bug 492279 we are improving Thunderbird's support for cookies. Whilst this is mainly for extension use, there are some cases, e.g. rss feeds where cookies may be presented to Thunderbird.

I think given we've got some cookie handling in Thunderbird and we multiple extensions may want cookies as well, we should be providing at least some minimal UI to manage cookies.

Uplifting the Firefox one direct would be hard as they have integrated it with privacy and history. However, it is worth flicking through their options.

I'm tempted to say that we should have at least two buttons: Exceptions and Show/Manager cookies. The reason being is that both of these allow management of things you can't manage via about:config (afaik).

Initially marking this as blocking and assigning to Bryan, as I'd like to get a resolution on what we are doing for TB 3 at least.

Comment 1

[Bryan Clark (DevTools PM) [:clarkbw]]

Adding bug 505530 as a dependency here because it was suggested in that bug that we pull the cookies UI into the security tab.

I think Mark is right on with the Exceptions and Show/Manager cookies buttons taking the user to dialogs similar to Firefox.

I'm going to assign this to blake to develop though magnus has already done the work on the security tab in bug 505530

Comment 2

[Bryan Clark (DevTools PM) [:clarkbw]]

Andreas and I will start work on some mockups for this

Comment 3

[Bryan Clark (DevTools PM) [:clarkbw]]

this isn't going to get much attention this week as we'll be focusing on bug 474711

If you're blocking on us we can start stealing from the Firefox Cookies UI and strip out the items that don't make sense.

Comment 4

[Andreas Nilsson (:andreasn)]

Attachment: screenshot of Firefox 3.5 cookies UI

Comment 5

[Bryan Clark (DevTools PM) [:clarkbw]]

Right on!  So perhaps in the network & disk tab we include a cookies section.

*Cookies*
[x] Accept cookies from sites        ( Exceptions... )
    Keep until ( they expire | v )   ( Show Cookies... )

Using at least the layout/look of the current dialogs from Firefox.  This would give us a minimal cookie interface, lifting some of what Firefox has done.  I'm not sure about the 'keep 3rd party cookies' option and if it should be included.

Comment 6

[Andreas Nilsson (:andreasn)]

We're going to run out of space if we add these settings to this tab. Looking over the dialog, we should be able to get rid of the two headers with only one thing beneath them (Connection and Offline), as it only repeats words and don't group several items.
We could also get rid of the icons in the Settings and 'Clear now' buttons (specific for Linux), will open as a new bug about that.
That would give us enough space to add these settings.

Comment 7

[Blake Winton (:bwinton) (:☕️)]

Attachment: A first cut at the patch.

For simplicity of implementation, I just copied over the javascript, and removed trailing whitespaces.  I didn't go through it and delete the functions we don't use.  I'm more than happy to, if either of you feel that it's important or useful.

Also, Bryan, I put the new items in a "Cookies" tab in the "Security" pane, as per comment #1, and after some discussion with Andreas.  We both agreed that you have the final say, but I figure it'll be easy enough to move them if you decide that they should be elsewhere, so I put them there for now.

Thanks,
Blake.

Comment 8

[Andreas Nilsson (:andreasn)]

some of the reasoning behind putting it in Security can be found on Wikipedia.
http://en.wikipedia.org/wiki/HTTP_cookie#Privacy_and_third-party_cookies

I find it more likely that someone wants to control cookie handling because of that, rather that they would run out of disk space due to too many cookies.

Comment 9

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 398997 [details] [diff] [review]
A first cut at the patch.

I'm a little uncertain about the cookies header.  Perhaps if we labeled it "Web Content" or "Remote Content" and then had the options for cookies in there.  "Cookies" is a term that doesn't have much to do with email and is highly technical in the web world.

Comment 10

[Blake Winton (:bwinton) (:☕️)]

(In reply to comment #9)
> (From update of attachment 398997 [details] [diff] [review])
> I'm a little uncertain about the cookies header.  Perhaps if we labeled it
> "Web Content" or "Remote Content" and then had the options for cookies in
> there. 

Changed to "Remote Content", because I don't know if people will associate their email with the web, even if the email is written in HTML.

Later,
Blake.

Comment 11

[Magnus Melin [:mkmelin]]

I'd prefer "Web Content" ;)
You can have remote content for mail too - like images, but cookies aren't applicable there.

I get this when opening the preferences 
Error: gSecurityPane.readAcceptCookies is not a function
Source File: chrome://global/content/bindings/preferences.xml
Line: 402

Don't forget about "Shredder should say something about cookies"

Comment 12

[Bryan Clark (DevTools PM) [:clarkbw]]

(In reply to comment #11)
> I'd prefer "Web Content" ;)
> You can have remote content for mail too - like images, but cookies aren't
> applicable there.

Good point, we do use "Remote Content" for our remote images notification so we could probably stick to "Web Content" here.

> Don't forget about "Shredder should say something about cookies"

I wanted to steal some prosaic text from Firefox about cookies and privacy but in my latest version they changed their cookie prefs UI to something else.  Will come up with something for this soon to dodge the string freeze; any suggestions?

Comment 13

[Blake Winton (:bwinton) (:☕️)]

Attachment: The previous patch, with mkmelin's and clarkbw's suggestions.

Okay, this should be better.

Thanks,
Blake.

Comment 14

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 399385 [details] [diff] [review]
The previous patch, with mkmelin's and clarkbw's suggestions.

looks good to me

Comment 15

[Magnus Melin [:mkmelin]]

I get this error

Error: acceptThirdParty is null
Source File: chrome://messenger/content/preferences/security.js
Line: 197

"Shredder can help you specify which blogs, news feeds, and other web sites are allowed to use cookies.": 

how about

"Shredder lets you specify which blogs, news feeds, and other web sites are allowed to set cookies."


+<!ENTITY showCookies.label              "Show Cookies…">
+<!ENTITY showCookies.accesskey          "S">
+
+

Remove the extra blank line.

+AtEndOfSession = at end of session

No spaces please (for consistency with the other ones)

+        windowtype="Mailnews:Permissions"

and 

+      document.documentElement.openWindow("mailnews:Permissions",

should match case. all lower case seems to be what's used elsewhere

Comment 16

[Magnus Melin [:mkmelin]]

Comment on attachment 399385 [details] [diff] [review]
The previous patch, with mkmelin's and clarkbw's suggestions.

Per previous comments

Comment 17

[Blake Winton (:bwinton) (:☕️)]

Attachment: The previous patch, with mkmelin's suggestions.

(In reply to comment #15)
> I get this error
> 
> Error: acceptThirdParty is null
> Source File: chrome://messenger/content/preferences/security.js
> Line: 197

Fixed.

> "Shredder can help you specify which blogs, news feeds, and other web sites are
> allowed to use cookies.": 
> 
> how about
> 
> "Shredder lets you specify which blogs, news feeds, and other web sites are
> allowed to set cookies."

Changed.

> +<!ENTITY showCookies.label              "Show Cookies…">
> +<!ENTITY showCookies.accesskey          "S">
> +
> +
> Remove the extra blank line.

Fixed.

> +AtEndOfSession = at end of session
> No spaces please (for consistency with the other ones)

Fixed.

> +        windowtype="Mailnews:Permissions"
> and 
> +      document.documentElement.openWindow("mailnews:Permissions",
> should match case. all lower case seems to be what's used elsewhere

Fixed.

Thanks,
Blake.

Comment 18

[Magnus Melin [:mkmelin]]

Comment on attachment 399522 [details] [diff] [review]
The previous patch, with mkmelin's suggestions.

r=mkmelin, with one problem to fix: the cookies dialog now has the title "Web Content". It should be "Cookies"

Comment 19

[Blake Winton (:bwinton) (:☕️)]

Attachment: The previous patch, with a better cookie window title.

> the cookies dialog now has the title "Web Content". It should be "Cookies"

Fixed.

Thanks,
Blake.

Comment 20

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/cbe9628bff57