1358943 - [Form Autofill] Support "address-line*" fields

Status: RESOLVED FIXED

(Toolkit :: Form Manager, enhancement, P3)

Description

[Luke Chang [:lchang] (inactive)]

Create "address-line1", "address-line2" and "address-line3" fields based on "street-address" when loading data from the storage and vice versa.

Comment 1

[Luke Chang [:lchang] (inactive)]

Attachment: Bug 1358943 - [Form Autofill] Support "address-line*" fields.

Review commit: https://reviewboard.mozilla.org/r/132756/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/132756/

Comment 2

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8860795 [details]
Bug 1358943 - [Form Autofill] Support "address-line*" fields.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/132756/diff/1-2/

Comment 3

[Matthew N. [:MattN]]

Comment on attachment 8860795 [details]
Bug 1358943 - [Form Autofill] Support "address-line*" fields.

https://reviewboard.mozilla.org/r/132756/#review137018

::: browser/extensions/formautofill/ProfileStorage.jsm:27
(Diff revision 2)
> + *       // extended fields
> + *       address-line1,
> + *       address-line2,
> + *       address-line3,

The comment at the top says that this is describing what's stored on disk but these won't be so I guess either remove these or change the comment.

::: browser/extensions/formautofill/ProfileStorage.jsm:27
(Diff revision 2)
>   *       postal-code,
>   *       country,          // ISO 3166
>   *       tel,
>   *       email,
>   *
> + *       // extended fields

Nit: I think "computed"/"calculated" fields is more clear. You may also want to note they're not saved.

::: browser/extensions/formautofill/ProfileStorage.jsm:299
(Diff revision 2)
> -      if (!searchString) {
> -        return !!name;
> -      }
> +    }
> +  },
>  
> -      return name.toLowerCase().startsWith(lcSearchString);
> +  _shrinkFields(profile) {

How about calling this `_normalizeAddress` first until we have other things to normalize.

::: browser/extensions/formautofill/ProfileStorage.jsm:311
(Diff revision 2)
> +        profile["address-line1"] = profile["street-address"];
> +        delete profile["street-address"];
> +      }
> +
> +      // Remove "address-line*" attributes but keep the values.
> +      let addressLine = [1, 2, 3].map(i => {

Nit: addressLines

::: browser/extensions/formautofill/test/unit/test_transformFields.js:53
(Diff revision 2)
> +      "street-address": "line1\nline2\nline3\nline4",
> +      "address-line1": "line1",
> +      "address-line2": "line2",
> +      "address-line3": "line3",

Can you file a bug to prevent the dataloss by concatenating with a locale-specific character in the future. I don't think it needs to be in the MVP though since this is an edgecase.

Comment 4

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8860795 [details]
Bug 1358943 - [Form Autofill] Support "address-line*" fields.

https://reviewboard.mozilla.org/r/132756/#review137018

Thanks.

> Can you file a bug to prevent the dataloss by concatenating with a locale-specific character in the future. I don't think it needs to be in the MVP though since this is an edgecase.

Filed bug 1334045.

Comment 5

[Luke Chang [:lchang] (inactive)]

(In reply to Luke Chang [:lchang] from comment #4)
> Filed bug 1334045.

Sorry. Should be bug 1360114.

Comment 6

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8860795 [details]
Bug 1358943 - [Form Autofill] Support "address-line*" fields.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/132756/diff/2-3/

Comment 7

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8860795 [details]
Bug 1358943 - [Form Autofill] Support "address-line*" fields.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/132756/diff/3-4/

Comment 8

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8860795 [details]
Bug 1358943 - [Form Autofill] Support "address-line*" fields.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/132756/diff/4-5/

Comment 9

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8860795 [details]
Bug 1358943 - [Form Autofill] Support "address-line*" fields.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/132756/diff/5-6/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/20109083820b
[Form Autofill] Support "address-line*" fields. r=MattN

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/20109083820b