security.js: Fix typos and bit-rot

RESOLVED FIXED in seamonkey2.0b1

Status

RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: philip.chee, Assigned: bugzilla)

Tracking

Trunk
seamonkey2.0b1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

10 years ago
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>
(Assignee)

Comment 1

10 years ago
Created attachment 366969 [details] [diff] [review]
Fix security.js

Here is a patch to fix the problems mentioned in comment 0.
Assignee: nobody → bugzilla
Status: NEW → ASSIGNED
Attachment #366969 - Flags: superreview?(neil)
Attachment #366969 - Flags: review?(neil)
(Assignee)

Updated

10 years ago
Blocks: 482650
(Reporter)

Updated

10 years ago
No longer blocks: 482650
Duplicate of this bug: 482650

Comment 3

10 years ago
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).
Attachment #366969 - Flags: superreview?(neil)
Attachment #366969 - Flags: superreview+
Attachment #366969 - Flags: review?(neil)
Attachment #366969 - Flags: review+
(Assignee)

Comment 4

10 years ago
(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.
Depends on: 483370
(Reporter)

Comment 5

10 years ago
> 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.
(Assignee)

Comment 6

10 years ago
Created attachment 367954 [details] [diff] [review]
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.
Attachment #366969 - Attachment is obsolete: true
Attachment #367954 - Flags: superreview?(neil)
Attachment #367954 - Flags: review?(neil)

Comment 7

10 years ago
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...)

Updated

10 years ago
Attachment #367954 - Flags: superreview?(neil)
Attachment #367954 - Flags: superreview+
Attachment #367954 - Flags: review?(neil)
Attachment #367954 - Flags: review+

Comment 8

10 years ago
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 ;-)
(Assignee)

Comment 9

10 years ago
Created attachment 368392 [details] [diff] [review]
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.
Attachment #367954 - Attachment is obsolete: true
Attachment #368392 - Flags: superreview+
Attachment #368392 - Flags: review+
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
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
Attachment #368392 - Attachment description: Fix security.js and port the filter code v1.1 → Fix security.js and port the filter code v1.1 [Checkin: Comment 10]
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0b1
You need to log in before you can comment on or make changes to this bug.