Closed
Bug 383993
Opened 17 years ago
Closed 17 years ago
remove p3p ui from suite
Categories
(SeaMonkey :: UI Design, defect)
SeaMonkey
UI Design
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: dwitte, Assigned: dwitte)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
kairo
:
review+
jag+mozilla
:
superreview+
|
Details | Diff | Splinter Review |
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.)
Assignee | ||
Comment 1•17 years ago
|
||
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.
Assignee: guifeatures → dwitte
Assignee | ||
Comment 2•17 years ago
|
||
this rips out the p3p stuff in prefs, as well as the cookie icon (which was exclusively p3p-related) and associated overlay.
Attachment #268635 -
Flags: superreview?(kairo)
Attachment #268635 -
Flags: review?(kairo)
Assignee | ||
Comment 3•17 years ago
|
||
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•17 years ago
|
||
Comment on attachment 268635 [details] [diff] [review]
remove chrome code
I'm no super-reviewer, Neil is though :)
Attachment #268635 -
Flags: superreview?(kairo) → superreview?(neil)
(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?
Assignee | ||
Comment 6•17 years ago
|
||
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.
Attachment #268635 -
Attachment is obsolete: true
Attachment #268635 -
Flags: superreview?(neil)
Attachment #268635 -
Flags: review?(kairo)
Assignee | ||
Comment 7•17 years ago
|
||
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.
Attachment #268726 -
Flags: superreview?(jag)
Attachment #268726 -
Flags: review?(kairo)
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++)
> {
Assignee | ||
Comment 9•17 years ago
|
||
(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•17 years ago
|
||
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.
Attachment #268726 -
Flags: review?(kairo) → review+
Comment 11•17 years ago
|
||
Everything looks good to me too now, sorry about the delay.
Updated•17 years ago
|
Attachment #268726 -
Flags: superreview?(jag) → superreview+
Assignee | ||
Comment 12•17 years ago
|
||
checked in, along with cvs removals.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment 13•17 years ago
|
||
-function viewCookiesFromIcon() {
- openCookieViewer("cookieManagerFromIcon");
-}
So people can no longer open the cookie viewer from the icon?
Assignee | ||
Comment 14•17 years ago
|
||
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•17 years ago
|
||
There's no longer a way to block certain domains from using cookies?
Assignee | ||
Comment 16•17 years ago
|
||
you have Tools->Cookies->Block Cookies from this site...
Comment 17•17 years ago
|
||
(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•17 years ago
|
||
There's also Tools|Cookie Manager|Allow Cookies from This Site
Comment 19•17 years ago
|
||
Couldn't we use the info bars currently ported in bug 270443 to notify users when cookies are blocked?
Comment 20•17 years ago
|
||
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•17 years ago
|
||
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.
You need to log in
before you can comment on or make changes to this bug.
Description
•