562183 - unify passphrase matching/validation code

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Mike Connor [:mconnor]]

Attachment: v1

This is inconsistent, and duplicates code, and also led to duplicates for a number of strings.

Comment 1

[Ed Lee :Mardak]

Missing fx-prefs-common.js ?

Comment 2

[Mike Connor [:mconnor]]

Attachment: v1

oops

Comment 3

[Ed Lee :Mardak]

Comment on attachment 441972 [details] [diff] [review]
v1

>+++ b/source/chrome/content/generic-change.js
>   validate: function (event) {
>     if (this._dialogType == "ChangePassword") {
>       if (this._currentPasswordInvalid)
>+        [valid, errorString] = gWeaveCommon.validatePassword(this._firstBox);
>+      else
>+        [valid, errorString] = gWeaveCommon.validatePassword(this._firstBox, this._secondBox);
>     }
>     else {
>       if (this._updatingPassphrase)
>+        [valid, errorString] = gWeaveCommon.validatePassphrase(this._firstBox);
>+      else
>+        [valid, errorString] = gWeaveCommon.validatePassphrase(this._firstBox, this._secondBox);
You could make the interface do..

let {valid, errorString} = gWeaveCommon.validatePassword(..)

Unrelated, not sure if it would be any better to do
.. = gWeaveCommon[validate].apply(gWeaveCommon, boxes)
where validate = dialogType == "Change" ? .. : .. and boxes = [first]; if (!invalid && !updating) boxes.push(second)

Mm.. maybe not :p

>+++ b/source/chrome/content/preferences/fx-prefs-common.js
>+var gWeaveCommon = {
var let?

>+  _validate: function (el1, el2, isPassword) {
>+    let errorString = error ? Weave.Utils.getErrorString(error) : "";
>+    dump("valid: " + valid + " error: " + errorString + "\n");
>+    return [valid, errorString];
And this would be:
return {
  errorString: errorString,
  valid: valid
};

>+++ b/source/chrome/content/preferences/fx-setup.js
>   onPasswordChange: function () {
>     let valid = false;
>+    let errorString = "";
>+    [valid, errorString] = gWeaveCommon.validatePassword(password, pwconfirm);
No need to separately declare/initialize and assign.

>+  _setFeedbackMessage: function (element, success, string) {
>-    label.className = classname;
No more error/success classes?

Comment 4

[Mike Connor [:mconnor]]

http://hg.mozilla.org/labs/weave/rev/6e4b48a00d01