Open Bug 1370391 Opened 5 years ago Updated 6 months ago

Include autofill heuristics profile emails in form manager results

Categories

(Toolkit :: Form Manager, enhancement, P3)

enhancement

Tracking

()

People

(Reporter: jonathanGB, Unassigned)

References

Details

Attachments

(1 file)

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 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,
```
Attachment #8876024 - Flags: review?(MattN+bmo) → review+
Priority: -- → P3
Assignee: jonathan.guillotte.blouin → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.