1400147 - [Form Autofill] Disable credit card heuristics when credit card availability is set to false

Status: RESOLVED FIXED

(Toolkit :: Form Manager, defect, P3)

Description

[Scott Wu [:scottwu]]

When `extensions.formautofill.creditCards.available` is set to `false`, we should skip running heuristics for credit card related fields, thus improve performance.

Comment 1

[Sean Lee [:seanlee][:weilonge]]

Attachment: Bug 1400147 - Ignore the credit card regexps when extensions.formautofill.creditCards.available is false.

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

Comment 2

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8910069 [details]
Bug 1400147 - Ignore the credit card regexps when extensions.formautofill.creditCards.available is false.

https://reviewboard.mozilla.org/r/181546/#review186938

::: browser/extensions/formautofill/FormAutofillHeuristics.jsm:490
(Diff revision 1)
> +      } else {
> +        if (!this._regexpListOf_CcUnavailable_AcOn) {
> +          this._regexpListOf_CcUnavailable_AcOn = regexps.filter(name => !FormAutofillUtils.isCreditCardField(name));
> +        }
> +        regexps = this._regexpListOf_CcUnavailable_AcOn;
> +      }

How about add an "early return" here when `regexps` is empty?

Comment 3

[Sean Lee [:seanlee][:weilonge]]

Comment on attachment 8910069 [details]
Bug 1400147 - Ignore the credit card regexps when extensions.formautofill.creditCards.available is false.

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

Comment 4

[Sean Lee [:seanlee][:weilonge]]

(In reply to Luke Chang [:lchang] from comment #2)
> How about add an "early return" here when `regexps` is empty?
Fix this in the latest patch.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6918dea06b7b
Ignore the credit card regexps when extensions.formautofill.creditCards.available is false. r=lchang

Comment 6

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

https://hg.mozilla.org/mozilla-central/rev/6918dea06b7b

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Backed out for heuristics xpcshell failures on Beta.
https://hg.mozilla.org/integration/mozilla-inbound/rev/540d10078b32f3f2426dc208df2fd710bb5c7ca1

https://treeherder.mozilla.org/logviewer.html#?job_id=132355777&repo=mozilla-beta

Comment 8

[Sean Lee [:seanlee][:weilonge]]

Comment on attachment 8910069 [details]
Bug 1400147 - Ignore the credit card regexps when extensions.formautofill.creditCards.available is false.

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

Comment 9

[Sean Lee [:seanlee][:weilonge]]

The patch is updated with the pref "extensions.formautofill.creditCards.available". Please help to land this patch again. Thanks.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ac393189541
Ignore the credit card regexps when extensions.formautofill.creditCards.available is false. r=lchang

Comment 11

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

https://hg.mozilla.org/mozilla-central/rev/7ac393189541

Comment 12

[Sean Lee [:seanlee][:weilonge]]

Comment on attachment 8910069 [details]
Bug 1400147 - Ignore the credit card regexps when extensions.formautofill.creditCards.available is false.

Approval Request Comment
[Feature/Bug causing the regression]: Feature
[User impact if declined]:
Credit Card feature is in a unnecessary code path, and this can have some performance impact.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: No.
[Is the change risky?]: No, this patch just adds a if-condition to ignore some regexps in available=false case.
[Why is the change risky/not risky?]:
[String changes made/needed]: No.

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8910069 [details]
Bug 1400147 - Ignore the credit card regexps when extensions.formautofill.creditCards.available is false.

Makes sense, taking it.
Should be in 57b5

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/dd4bcf227b95

Comment 15

[Andrei Vaida [:avaida]]

(In reply to Sean Lee [:seanlee][:weilonge] from comment #12)
> [Is this code covered by automated tests?]: Yes.
> [Has the fix been verified in Nightly?]: Yes.
> [Needs manual test from QE? If yes, steps to reproduce]: No.

Setting qe-verify- based on Sean's assessment on manual testing needs and the fact that this fix has automated coverage.