476274 - security.js: Fix typos and bit-rot

Status: RESOLVED FIXED

(SeaMonkey :: Page Info, defect)

Description

[Philip Chee]

While triaging Bug 461574 I noticed several problems:

1. "Browser:Cookies" is C&P from Firefox. Our window type is "mozilla:cookieviewer"
<http://mxr.mozilla.org/comm-central/source/suite/browser/pageinfo/security.js#141>

2. "chrome://communicator/content/wallet/SignonViewer.xul" doesn't exist and has been replaced by "chrome://communicator/content/passwordManager.xul"
<http://mxr.mozilla.org/comm-central/source/suite/browser/pageinfo/security.js#149>

Comment 1

[Krang]

Attachment: Fix security.js

Here is a patch to fix the problems mentioned in comment 0.

Comment 3

[[e-mail]]

Comment on attachment 366969 [details] [diff] [review]
Fix security.js

>   viewPasswords: windowOpener("Toolkit:PasswordManager",
>-                              "chrome://communicator/content/wallet/SignonViewer.xul",
>+                              "chrome://communicator/content/passwordManager.xul",
>                               "S"),
I don't suppose you could port the rest of the viewPasswords code from browser/base/content/pageinfo/security.js? (I think the cookie viewer might need some tweaking before you can port the viewCookies part).

Comment 4

[Krang]

(In reply to comment #3)
> I don't suppose you could port the rest of the viewPasswords code from
> browser/base/content/pageinfo/security.js? (I think the cookie viewer might
> need some tweaking before you can port the viewCookies part).

Sure, I can update the patch to take care of that, but as I was testing it, I realized that Firefox's code is broken.  I filed bug 483370 to cover that.

Comment 5

[Philip Chee]

> Sure, I can update the patch to take care of that,
Or you could keep this bug open and attach a follo-wup patch after this gets checked into the tree.

Comment 6

[Krang]

Attachment: Fix security.js and port the filter code

Now that bug 483370 is fixed, here is the updated patch.  I also updated the cookie viewer to allow the filter to be set.

(In reply to comment #5)
> Or you could keep this bug open and attach a follo-wup patch after this gets
> checked into the tree.

Good idea, but I thought it would probably be better to wait since the new patch completely replaces the old one.

Comment 7

[[e-mail]]

Comment on attachment 367954 [details] [diff] [review]
Fix security.js and port the filter code

>+      window.openDialog("chrome://communicator/content/permissions/cookieViewer.xul",
>+                        "mozilla:cookieviewer", "", {filterString : eTLD});

>+      window.openDialog("chrome://communicator/content/passwordManager.xul",
>+                        "Toolkit:PasswordManager", "", 
>+                        {filterString : this._getSecurityInfo().hostName});
Haven't actually tried the patch yet but at a quick glance these look wrong; the second parameter should actually be blank (this was a bug in the deleted code too; it generates a warning in debug builds) and the third parameter should probably be "chrome,resizable" or "all,dialog=no" (we're inconsistent...)

Comment 8

[[e-mail]]

Comment on attachment 367954 [details] [diff] [review]
Fix security.js and port the filter code

Ah, so no features at all is wrong, because then you get a window with min/max buttons (and also a Mac toolbar collapse button). r+sr=me with that fixed.

I guess I need to file a bug on the Password Manager menuitem, it should be using "chrome,resizable" too :-(

>+                        "Toolkit:PasswordManager", "", 
Oh, and there's a trailing space here too ;-)

Comment 9

[Krang]

Attachment: Fix security.js and port the filter code v1.1 [Checkin: Comment 10]

Thanks for the reviews.  Here is the patch for checkin.

(In reply to comment #8)
> Ah, so no features at all is wrong, because then you get a window with min/max
> buttons (and also a Mac toolbar collapse button). r+sr=me with that fixed.
> 
> I guess I need to file a bug on the Password Manager menuitem, it should be
> using "chrome,resizable" too :-(

Fixed.  I used "resizable" here to match bug 483960.

> Oh, and there's a trailing space here too ;-)

Fixed.

Comment 10

[Serge Gautherie (:sgautherie)]

Comment on attachment 368392 [details] [diff] [review]
Fix security.js and port the filter code v1.1
[Checkin: Comment 10]


http://hg.mozilla.org/comm-central/rev/9b8d068b160b