New issue
Advanced search Search tips

Issue 880552 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug
M70



Sign in to add a comment

Autofill setting page used obsolete pref and led to undefined behavior

Project Member Reported by jiahuiguo@chromium.org, Sep 4

Issue description

Cc: dlkumar@google.com
After some investigation, the culprit is we are using the wrong pref field 
prefs.autofill.enabled.value, instead of prefs.autofill.profile_enabled.value.

This also affects the toggle for "Add" button, where even we flip the toggle, the "Add" button is not disabled as expected. [1]

https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/passwords_and_forms_page/payments_section.html?l=64-73&rcl=f2f5ee50a0a9c936dd76d85aafcab7146fc7d2de




Screenshot from 2018-09-04 17-16-26.png
17.4 KB View Download


Before toggle.png
30.0 KB View Download
After Toggle.png
22.1 KB View Download
Are you not looking for the "credit card enabled" preference?
Hi Mathieu, we are supposed to look for both and call 

eitherIsDisabled_(prefs.autofill.enabled.value,prefs.autofill.credit_card_enabled.value)

But since prefs.autofill.enabled is not defined when using prefs.autofill.enabled.value, it will blown up, nothing is evaluated and the behavior is undefined in this case. (Add is not disabled and migration button shows up)
The master preference (autofill.enabled) is no longer used. You could only check credit_card_enabled.


Project Member

Comment 7 by bugdroid1@chromium.org, Sep 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/45e35ec4e91b327307ab8e0bb896ce08e5381d7a

commit 45e35ec4e91b327307ab8e0bb896ce08e5381d7a
Author: Jason Guo <jiahuiguo@chromium.org>
Date: Wed Sep 05 22:04:50 2018

[Autofill] Fix pref name in settings.

Details please refer to the bug threads.

Bug:  880552 ,  880833 
Change-Id: I90705523da9c513a70990647915422b574b02753
Reviewed-on: https://chromium-review.googlesource.com/1205536
Reviewed-by: Hector Carmona <hcarmona@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Commit-Queue: Jason Guo <jiahuiguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589019}
[modify] https://crrev.com/45e35ec4e91b327307ab8e0bb896ce08e5381d7a/chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html
[modify] https://crrev.com/45e35ec4e91b327307ab8e0bb896ce08e5381d7a/chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.js
[modify] https://crrev.com/45e35ec4e91b327307ab8e0bb896ce08e5381d7a/chrome/browser/resources/settings/passwords_and_forms_page/payments_section.html
[modify] https://crrev.com/45e35ec4e91b327307ab8e0bb896ce08e5381d7a/chrome/browser/resources/settings/passwords_and_forms_page/payments_section.js
[modify] https://crrev.com/45e35ec4e91b327307ab8e0bb896ce08e5381d7a/chrome/test/data/webui/settings/autofill_section_test.js
[modify] https://crrev.com/45e35ec4e91b327307ab8e0bb896ce08e5381d7a/chrome/test/data/webui/settings/payments_section_test.js

Labels: Merge-Request-70
Cc: mahmadi@chromium.org hcarmona@chromium.org
Labels: -Pri-2 Pri-1
Summary: Autofill setting page used obsolete pref and led to undefined behavior (was: Migration button is not hidden properly even if experiment flag is off)
Project Member

Comment 12 by sheriffbot@chromium.org, Sep 7

Labels: -Merge-Request-70 Hotlist-Merge-Approved Merge-Approved-70
Your change meets the bar and is auto-approved for M70. Please go ahead and merge the CL to branch 3538 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Fixed (was: Assigned)
Project Member

Comment 14 by bugdroid1@chromium.org, Sep 7

Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5a3d5a73a76fd3a1c02df491461eb42b636f5bea

commit 5a3d5a73a76fd3a1c02df491461eb42b636f5bea
Author: Mathieu Perreault <mathp@chromium.org>
Date: Fri Sep 07 00:29:25 2018

[Autofill] Fix pref name in settings.

Details please refer to the bug threads.

TBR=jiahuiguo@chromium.org

(cherry picked from commit 45e35ec4e91b327307ab8e0bb896ce08e5381d7a)

Bug:  880552 ,  880833 
Change-Id: I90705523da9c513a70990647915422b574b02753
Reviewed-on: https://chromium-review.googlesource.com/1205536
Reviewed-by: Hector Carmona <hcarmona@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Commit-Queue: Jason Guo <jiahuiguo@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#589019}
Reviewed-on: https://chromium-review.googlesource.com/1211920
Cr-Commit-Position: refs/branch-heads/3538@{#120}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/5a3d5a73a76fd3a1c02df491461eb42b636f5bea/chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html
[modify] https://crrev.com/5a3d5a73a76fd3a1c02df491461eb42b636f5bea/chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.js
[modify] https://crrev.com/5a3d5a73a76fd3a1c02df491461eb42b636f5bea/chrome/browser/resources/settings/passwords_and_forms_page/payments_section.html
[modify] https://crrev.com/5a3d5a73a76fd3a1c02df491461eb42b636f5bea/chrome/browser/resources/settings/passwords_and_forms_page/payments_section.js
[modify] https://crrev.com/5a3d5a73a76fd3a1c02df491461eb42b636f5bea/chrome/test/data/webui/settings/autofill_section_test.js
[modify] https://crrev.com/5a3d5a73a76fd3a1c02df491461eb42b636f5bea/chrome/test/data/webui/settings/payments_section_test.js

Cc: jiahuiguo@chromium.org
 Issue 883128  has been merged into this issue.

Sign in to add a comment