383993 - remove p3p ui from suite

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[[e-mail]]

followup to bug 366611 - now that the p3p backend has been removed, the suite ui that goes with it should be cleaned up. i don't know suite all that well, so i'd rather someone more familiar with it take care of this. it approximately covers the files:

all of
suite/common/permissions/cookieP3PDialog.xul
suite/common/permissions/cookieP3P.xul
suite/locales/en-US/chrome/common/permissions/cookieP3P.dtd
most of
suite/common/permissions/cookieTasksOverlay.xul
and some of
suite/common/pref/pref-cookies.xul

plus other random stuff in the help files. grep for 'p3p' in suite and most of it can be found. (as a side note, the cookieIcon stuff is also exclusively p3p-related and should be ditched along with p3p ui.)

Comment 1

[[e-mail]]

i have a patch in hand to remove the chrome code. i haven't touched the help files; these will need to be updated and i'll file a followup bug for this.

Comment 2

[[e-mail]]

Attachment: remove chrome code

this rips out the p3p stuff in prefs, as well as the cookie icon (which was exclusively p3p-related) and associated overlay.

Comment 3

[[e-mail]]

note: most of the patch is just removal of the following files, so don't be put off by the size ;)

suite/common/permissions/cookieP3PDialog.xul
suite/common/permissions/cookieP3P.xul
suite/locales/en-US/chrome/common/permissions/cookieP3P.dtd
suite/common/permissions/cookieTasksOverlay.xul
suite/locales/en-US/chrome/common/permissions/cookieTasksOverlay.dtd

Comment 4

[Robert Kaiser]

Comment on attachment 268635 [details] [diff] [review]
remove chrome code

I'm no super-reviewer, Neil is though :)

Comment 5

[Ian Neal]

(In reply to comment #2)
> Created an attachment (id=268635) [details]
> remove chrome code
> 
> this rips out the p3p stuff in prefs, as well as the cookie icon (which was
> exclusively p3p-related) and associated overlay.
> 
You will need to remove some code from cookieViewer.xul/js as well http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/suite/common/permissions/cookieViewer.js&mark=200-218 or will that be covered in bug 383994?

Comment 6

[[e-mail]]

Comment on attachment 268635 [details] [diff] [review]
remove chrome code

you're right; i think i'd rather do that here so i can get all the chrome fixes done in one batch. new patch coming up.

Comment 7

[[e-mail]]

Attachment: v2

same as before, but this patch includes changes to cookieviewer to remove the "policy" and "status" flags. all the changes in this patch are simple code removal.

Comment 8

[Ian Neal]

Comment on attachment 268726 [details] [diff] [review]
v2

>Index: suite/common/permissions/cookieViewer.js
>===================================================================
>@@ -311,13 +246,12 @@ function CookieSelected() {
>     {id: "ifl_path", value: cookies[idx].path},
>     {id: "ifl_isSecure",
>      value: cookies[idx].isSecure ?
>             cookieBundle.getString("forSecureOnly") : 
>             cookieBundle.getString("forAnyConnection")},
>     {id: "ifl_expires", value: cookies[idx].expires},
Extraneous comma?

>-    {id: "ifl_policy", value: GetPolicyString(cookies[idx].policy)}
>   ];
> 
>   var value;
>   var field;
>   for (var i = 0; i < props.length; i++)
>   {

Comment 9

[[e-mail]]

(In reply to comment #8)
> Extraneous comma?

thanks, i'll fix that during checkin (not worth posting a new patch).

btw, feel free to jump in and mark your review on this, since it seems you've already read through it a couple times. i'm sure kairo won't mind ;)

Comment 10

[Robert Kaiser]

Comment on attachment 268726 [details] [diff] [review]
v2

This builds well for me, nothing looks broken in the UI, the stuff that should be gone is gone, and the removals look correct to my eyes.
r=me as long as jag checks that the removals in the xul/js parts are correct, as I still feel I know too little of this code to give it a thorough review.

Comment 11

[Ian Neal]

Everything looks good to me too now, sorry about the delay.

Comment 12

[[e-mail]]

checked in, along with cvs removals.

Comment 13

[mv_van_rantwijk]

-function viewCookiesFromIcon() {
-  openCookieViewer("cookieManagerFromIcon");
-}  

So people can no longer open the cookie viewer from the icon?

Comment 14

[[e-mail]]

there no longer is a cookie icon, because there's no longer anything to flag cookies and throw the icon in the first place.

Comment 15

[mv_van_rantwijk]

There's no longer a way to block certain domains from using cookies?

Comment 16

[[e-mail]]

you have Tools->Cookies->Block Cookies from this site...

Comment 17

[mv_van_rantwijk]

(In reply to comment #16)
> you have Tools->Cookies->Block Cookies from this site...

So there's no longer a visual clue as to if cookies are being blocked, and no easy way to change this behavior when they are.  Great thinking!

Comment 18

[Christian :Biesinger (don't email me, ping me on IRC)]

There's also Tools|Cookie Manager|Allow Cookies from This Site

Comment 19

[Bruno 'Aqualon' Escherl]

Couldn't we use the info bars currently ported in bug 270443 to notify users when cookies are blocked?

Comment 20

[mv_van_rantwijk]

I know that, but you could also have added the: Tools -> Cookie Manager popup to the cookie icon, which would have made it far more convenient.

What next?  The removal of the blocked popup button?

Comment 21

[Robert Kaiser]

The cookie icon only ever worked with P3P-based cookie policies, not for normal cookie blocking, as far as I can tell - we might have taken a patch that would have changed that but nobody did car to write one.

I think the idea from comment #19 might be interesting, though.