1370391 - Include autofill heuristics profile emails in form manager results

Status: NEW

(Toolkit :: Form Manager, enhancement, P3)

Description

[Jonathan Guillotte-Blouin [:jonathanGB]]

Bug 1364602 included autofill profile emails in form manager results, but only when the focused input was specifically of type="email".

The next step is to integrate the heuristics to include such autofill profile emails to inputs which are not using the type="email" but ought to be considered as emails.

Comment 1

[Jonathan Guillotte-Blouin [:jonathanGB]]

Attachment: Bug 1370391 - integrate heuristics into results.

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

Comment 2

[Jonathan Guillotte-Blouin [:jonathanGB]]

Comment on attachment 8876024 [details]
Bug 1370391 - integrate heuristics into results.

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

Comment 3

[Matthew N. [:MattN]]

Comment on attachment 8876024 [details]
Bug 1370391 - integrate heuristics into results.

https://reviewboard.mozilla.org/r/147470/#review159978

::: toolkit/components/satchel/nsFormAutoComplete.js:17
(Diff revision 2)
> +  try {
> +    _formAutofillContent = Cu.import("resource://formautofill/FormAutofillContent.jsm", {})
> +                             .FormAutofillContent;
> +  } catch (e) {

Maybe add a comment of why we're not using defineLazyModuleGetter. e.g.:

Gracefully handle if the Form Autofill System Add-on isn't installed in this locale or for this user. defineLazyModuleGetter would report to the console.

::: toolkit/components/satchel/nsFormAutoComplete.js:517
(Diff revision 2)
>     *  callback - called when the values are available. Passed an array of objects,
>     *             containing properties for each result. The callback is only called
>     *             when successful.
>     */
>    getAutoCompleteValues(client, aField, searchString, callback) {
> +    let heuristics = (!aField.hasOwnProperty("noHeuristics") && FormAutofillContent) ?

I think you can get rid of the `noHeuristics` property and use `aField instanceof Ci.nsIDOMHTMLInputElement` here instead. You should probably update the jsDoc comment to indicate that aField is sometimes a vanilla JSObject. e.g.:

@param {nsIDOMHTMLInputElement|object} aField - …

::: toolkit/components/satchel/nsFormAutoComplete.js:530
(Diff revision 2)
>        fieldtype:          aField.type,
>        maxTimeGroupings:   this._maxTimeGroupings,
>        timeGroupingSize:   this._timeGroupingSize,
>        prefixWeight:       this._prefixWeight,
>        boundaryWeight:     this._boundaryWeight,
> +      heuristicsFieldType: (heuristics === null ? null : heuristics.fieldName),

Nit: You don't need the braces or the `=== null` check as a truthiness check on heuristics is enough:

```js
heuristicsFieldType: heuristics ? heuristics.fieldName : null,
```

Comment 4

[Jonathan Guillotte-Blouin [:jonathanGB]]

Comment on attachment 8876024 [details]
Bug 1370391 - integrate heuristics into results.

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

Comment 5

[Jonathan Guillotte-Blouin [:jonathanGB]]

Comment on attachment 8876024 [details]
Bug 1370391 - integrate heuristics into results.

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

Comment 6

[Jonathan Guillotte-Blouin [:jonathanGB]]

Comment on attachment 8876024 [details]
Bug 1370391 - integrate heuristics into results.

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

Comment 7

[Jonathan Guillotte-Blouin [:jonathanGB]]

Comment on attachment 8876024 [details]
Bug 1370391 - integrate heuristics into results.

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