Closed Bug 444170 Opened 13 years ago Closed 12 years ago

Migrate SeaMonkey's Master Password preferences to new pref window

Categories

(SeaMonkey :: Preferences, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.0a1

People

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

References

Details

Attachments

(1 file, 1 obsolete file)

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
Target Milestone: --- → seamonkey2.0alpha
Version: unspecified → Trunk
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.
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.
No longer depends on: 390025
Attached patch First patch (obsolete) — Splinter Review
Attachment #337443 - Flags: superreview?(neil)
Attachment #337443 - Flags: review?(neil)
Attachment #337443 - Flags: superreview?(neil)
Attachment #337443 - Flags: review?(neil)
Attachment #337443 - Flags: review-
Comment on attachment 337443 [details] [diff] [review]
First patch

Weird, this pane didn't load up corretly, although I can't see what's wrong.

>-<!--
>           <treeitem id="masterpassItem"
>                     label="&masterpass.label;"
>                     prefpane="masterpass_pane"
>                     url="chrome://pippki/content/pref-masterpass.xul"
>                     helpTopic="passwords_master"/>
>--->
Nit: Also needs to be marked as migrated in PrefOverlay.xul

>+  switch(gInternalToken.getAskPasswordTimes()) {
Nit: space between switch and (

+    value = 2
Nit: missing ;

>+function WriteAskForPassword(field) {
>+  var asktimes;
Nit: I preferred the old askTimes (uppercase T)

>+  switch(field.value) {
Nit: space again

>+  prefExpire.value = expire;
Could use field.value == "1" here.

>+  return Number(field.value);
I don't think you need to do this, the preference widget includes conversions.

>+      <hbox align="center">
Nit: don't need this align.

>+      <hbox align="center">
This box is pointless, all it contains is a radiogroup, which is a container.

>+        <radiogroup id="passwordAskTimes"
>+                    flex="1"
And then you can get rid of this flex too.

>+                 style="margin: 0px;"/>
>+                 style="margin: 0px;"/>
>+                   style="margin: 0px;"/>
>+                   style="margin: 4px;"/>
How odd. Please get rid of them, they're ugly!

>+          <hbox align="center">
Ah, now this align is correct ;-)

>+      <hbox align="center">
But you don't need this one either.
Comment on attachment 337443 [details] [diff] [review]
First patch

> function ChangePW()
> {
>+  var p = Components.classes["@mozilla.org/embedcomp/dialogparam;1"]
>+                    .createInstance(Components.interfaces.nsIDialogParamBlock);
>+  p.SetString(1, "");
>   window.openDialog("chrome://pippki/content/changepassword.xul","",
Nit: space needed before ""
>+                    "chrome,centerscreen,modal", p);
> }
> 
>+          <radio id="askFirstTime"
As far as I can see you do not need these first two radio ids anymore.
>+                 value="0"
>+                 label="&managepassword.askfirsttime;"
>+                 accesskey="&managepassword.askfirsttime.accesskey;"
>+                 style="margin: 0px;"/>
>+          <radio id="askEveryTime"
>+                 value="1"
>+                 label="&managepassword.askeverytime;"
>+                 accesskey="&managepassword.askeverytime.accesskey;"
>+                 style="margin: 0px;"/>
Attached patch Second patchSplinter Review
Patch with above nits fixed
Attachment #337443 - Attachment is obsolete: true
Attachment #337542 - Flags: superreview?(neil)
Attachment #337542 - Flags: review?(neil)
Comment on attachment 337542 [details] [diff] [review]
Second patch

This version seems to work fine.
Attachment #337542 - Flags: superreview?(neil)
Attachment #337542 - Flags: superreview+
Attachment #337542 - Flags: review?(neil)
Attachment #337542 - Flags: review+
Keywords: push-needed
Checked in, changeset id: 298:adbb63fc38c9
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.