Closed Bug 444169 Opened 12 years ago Closed 12 years ago

Migrate SeaMonkey's Password preferences to new pref window

Categories

(SeaMonkey :: Preferences, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.0a1

People

(Reporter: Manuel.Spam, Assigned: Manuel.Spam)

References

Details

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.13) Gecko/20080328 SeaMonkey/1.1.9
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.13) Gecko/20080328 SeaMonkey/1.1.9

No need for much text here...

Reproducible: Always
I want to take this one and publish a patch (even if this ... Bugzilla doesn't
allow me to take the bug).
Assignee: nobody → Manuel.Spam
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: unspecified → Trunk
Target Milestone: --- → seamonkey2.0alpha
Status: NEW → ASSIGNED
Depends on: 390025
Please do not block this on bug 390025, as the wallet migration possibly won't make the first alpha, but the pref migration (bug 394522) needs to be done for this alpha, even if that means adjusting this panel again when bug 390025 has landed.
Depends on: 450257
We have set a deadline of September 9 to have all prefpanels done. If you can't have a patch ready for checkin or until for review within those two weeks, please unassign and find someone who can do the patch in time.
Attached patch First patch (obsolete) — Splinter Review
Attachment #337412 - Flags: superreview?(neil)
Attachment #337412 - Flags: review?(neil)
No longer depends on: 390025
Comment on attachment 337412 [details] [diff] [review]
First patch

>+function initReencryptCallback()
>+{
>+  Components.classes['@mozilla.org/wallet/wallet-service;1']
>+            .getService()
>+            .QueryInterface(Components.interfaces.nsIWalletService)
>+            .WALLET_InitReencryptCallback(window);
>+}
Eww, wallet sucks. You can't actually safely handle this. Just remove it (and all the UI for the wallet.crypto pref). So in fact we have to go with KaiRo's attachment 301676 [details] [diff] [review] anyway. Sorry about that.

Actually that attachment doesn't quite work, as it depends on toPasswordManager which isn't implemented yet (see his attachment 301310 [details] [diff] [review]), so what we need is a temporary toPasswordManager function in utilityOverlay.js which uses the code from viewSignons.
Attachment #337412 - Flags: superreview?(neil)
Attachment #337412 - Flags: review?(neil)
Comment on attachment 337412 [details] [diff] [review]
First patch

>+function initReencryptCallback()
>+{
>+  Components.classes['@mozilla.org/wallet/wallet-service;1']
>+            .getService()
>+            .QueryInterface(Components.interfaces.nsIWalletService)
>+            .WALLET_InitReencryptCallback(window);
>+}
So, as we discussed on IRC, the compromise is to do this in Startup.

>+ The Original Code is Mozilla Communicator client code, released
>+ March 31, 1998.
I notice that KaiRo's licence block looks slightly different. Maybe the boilerplate changed since 1998?

>+    <preferences>
We like to give this an id for ease of extension overlays (see KaiRo's patch).

>+      <description flex="1">&signonDescription.label;</description>
>+      <hbox align="start">
Nits: The flex isn't necessary. The align is unnecessary as it's an hbox.

>+        <button label="&viewSignons.label;"
>+                accesskey="&viewSignons.accesskey;"
>+                oncommand="viewSignons();"
>+                id="viewStoredPassword"
id first please.
Attached patch Second patch (obsolete) — Splinter Review
Attachment #337412 - Attachment is obsolete: true
Attachment #337449 - Flags: superreview?(neil)
Attachment #337449 - Flags: review?(neil)
Attachment #337449 - Flags: superreview?(neil)
Attachment #337449 - Flags: superreview+
Attachment #337449 - Flags: review?(neil)
Attachment #337449 - Flags: review+
Comment on attachment 337449 [details] [diff] [review]
Second patch

>+            .getService()
>+            .QueryInterface(Components.interfaces.nsIWalletService)
Nit: Sorry for not spotting this before, but you can write this:
.getService(Components.interfaces.nsIWalletService)

>+    <preferences>
Nit: id="passwords_preferences"
Comment on attachment 337449 [details] [diff] [review]
Second patch

>+++ b/suite/common/pref/pref-passwords.js	Mon Sep 08 17:31:54 2008 +0200
>+function viewSignons()
Function names should start with a capital letter.

>+++ b/suite/common/pref/pref-passwords.xul	Mon Sep 08 17:31:54 2008 +0200
>+        <button id="viewStoredPassword"
>+                label="&viewSignons.label;"
>+                accesskey="&viewSignons.accesskey;"
>+                oncommand="viewSignons();"
See above.
>+                preference="pref.advanced.password.disable_button.view_stored_password"/>
Version with nits fixed. Forwarding r+ and sr+
Attachment #337449 - Attachment is obsolete: true
Attachment #337541 - Flags: superreview+
Attachment #337541 - Flags: review+
Keywords: push-needed
Comment on attachment 337541 [details] [diff] [review]
Patch with nits fixed (Checkin: Comment 11)

http://hg.mozilla.org/comm-central/rev/0f7442ea15db
Attachment #337541 - Attachment description: Patch with nits fixed → Patch with nits fixed (Checkin: Comment 11)
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Keywords: push-needed
Resolution: --- → FIXED
It looks like this fix includes support for encryption and obscuring, although I'd heard that functionality was no longer available. I think Kairo originally had a plan to omit it and then merge the one lonely check box left here with Master Passwords to make a single pref pane.
Simon, that all applies only after the new password manager backend has landed and wallet is killed, which we couldn't do yet because of a number of fixes needed in mailnews code to work with that new backend. See bug 390025.
You need to log in before you can comment on or make changes to this bug.