Sort the profile results for autocomplete popup by last used time

RESOLVED FIXED in Firefox 56

Status

()

enhancement
P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: selee, Assigned: steveck)

Tracking

(Blocks 1 bug)

unspecified
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [form autofill:M3])

Attachments

(1 attachment)

After having the last used time in ProfileStorage, the profiles got from parent should contain the usage information and sorted by the last used time.
Whiteboard: [form autofill:M1] → [form autofill:MVP]
Whiteboard: [form autofill:MVP] → [form autofill:M3]
Assignee

Updated

2 years ago
Assignee: nobody → schung

Comment 2

2 years ago
mozreview-review
Comment on attachment 8879489 [details]
Bug 1339745 - Sort the profile results for autocomplete popup by last used time,

https://reviewboard.mozilla.org/r/150796/#review155584

::: browser/extensions/formautofill/FormAutofillContent.jsm:115
(Diff revision 1)
>      this._getAddresses({info, searchString}).then((addresses) => {
>        if (this.forceStop) {
>          return;
>        }
> +      // Sort addresses by timeLastUsed for showing the lastest used address at top.
> +      addresses.sort((a, b) => b.timeLastUsed - a.timeLastUsed);

Should we add the second factor if two addresses are added but not used? Maybe `timeCreated`?

::: browser/extensions/formautofill/test/mochitest/test_on_address_submission.html:32
(Diff revision 1)
> -// Autofill the address from dropdown menu.
> +let expectingPopup = null;
> +function expectPopup() {
> +  info("expecting a popup");
> +  return new Promise(resolve => {
> +    expectingPopup = resolve;
> +  });
> +}
> +function popupShownListener() {
> +  info("popup shown for test ");
> +  if (expectingPopup) {
> +    expectingPopup();
> +    expectingPopup = null;
> +  }
> +}

Can these be shared from common.js?
Attachment #8879489 - Flags: review?(lchang)
Comment hidden (mozreview-request)
Assignee

Comment 4

2 years ago
mozreview-review-reply
Comment on attachment 8879489 [details]
Bug 1339745 - Sort the profile results for autocomplete popup by last used time,

https://reviewboard.mozilla.org/r/150796/#review155584

> Should we add the second factor if two addresses are added but not used? Maybe `timeCreated`?

Since UX didn't really care about the sorting for addresses with same last used time, maybe we can ignore it unless user complains.

Comment 5

2 years ago
mozreview-review
Comment on attachment 8879489 [details]
Bug 1339745 - Sort the profile results for autocomplete popup by last used time,

https://reviewboard.mozilla.org/r/150796/#review156672

::: browser/extensions/formautofill/test/mochitest/formautofill_common.js:112
(Diff revision 2)
> +function popupShownListener() {
> +  info("popup shown for test ");
> +  if (expectingPopup) {
> +    expectingPopup();
> +    expectingPopup = null;
> +  }
> +}

Although I know it's unrelated to your patch, could you futher facilitate this util function by adding something like: 

```js
function initPopupListener() {
  registerPopupShownListener(popupShownListener);
}
```

Thanks.

::: browser/extensions/formautofill/test/mochitest/test_on_address_submission.html:62
(Diff revision 2)
> +    await setInput("#" + key, TEST_ADDRESSES[1][key]);
> +  }
> +
> +  clickOnElement("input[type=submit]");
> +
> +  // The 2nd test address should be on the top since it's the latest one.

nit: "... since it's the last used one".
Attachment #8879489 - Flags: review?(lchang) → review+
Comment hidden (mozreview-request)
Assignee

Comment 7

2 years ago
Thanks!
Keywords: checkin-needed

Comment 8

2 years ago
Pushed by lchang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/26c2121cc1ef
Sort the profile results for autocomplete popup by last used time, r=lchang
Keywords: checkin-needed

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/26c2121cc1ef
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.