New issue
Advanced search Search tips

Issue 914944 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Dec 17
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug



Sign in to add a comment

[Autofill Sync] Opt-in prompt appears on CVC fields

Project Member Reported by tmartino@chromium.org, Dec 13

Issue description

The logic determining whether or not to show the Autofill Sync Transport prompt in the dropdown incorrectly includes CVC fields. Clicking the prompt in this case leads to a DCHECK; we don't normally offer the dropdown on CVC fields, and so no cards are available to show when the dropdown is re-displayed.

The fix here is simply to be more stringent about which fields are included for the prompt.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 13

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

commit 3b5fea2584edb0bb8b155bed9cc386aa3f6c66c6
Author: Tommy Martino <tmartino@chromium.org>
Date: Thu Dec 13 22:33:45 2018

[Autofill Sync] Do not show downstream prompt on CVC fields

This CL fixes a small bug where the opt-in prompt for Autofill Sync
Transport is shown on CVC and CC type fields. This is necessary because
these fields don't actually have suggestions available, so we hit a
DCHECK when the user opts in and we try to show them suggestions.

Bug:  914944 
Change-Id: I38061b59699de623d0d828d1932dbfd1ad7aceee
Reviewed-on: https://chromium-review.googlesource.com/c/1376813
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Tommy Martino <tmartino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616459}
[modify] https://crrev.com/3b5fea2584edb0bb8b155bed9cc386aa3f6c66c6/components/autofill/core/browser/autofill_manager.cc

Labels: butter-hotlist
Labels: Merge-Request-72
Status: Verified (was: Started)
Verified in Canary this AM.

Requesting merge to M-72. This CL fixes a small bug that triggered a DCHECK under common circumstances and could lead to unexpected behavior in non-Debug builds. It is extremely small and very low-risk; the CL simply adds two enum equality checks to a single early-return condition.
Project Member

Comment 4 by sheriffbot@chromium.org, Dec 17

Labels: -Merge-Request-72 Merge-Review-72 Hotlist-Merge-Review
This bug requires manual review: M72 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-72 Merge-Approved-72
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 17

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/71cadb33ce6466480b127b7b65bf1e9a27bbc506

commit 71cadb33ce6466480b127b7b65bf1e9a27bbc506
Author: Tommy Martino <tmartino@chromium.org>
Date: Mon Dec 17 18:54:23 2018

[Merge 72][Autofill Sync] Do not show downstream prompt on CVC fields

This CL fixes a small bug where the opt-in prompt for Autofill Sync
Transport is shown on CVC and CC type fields. This is necessary because
these fields don't actually have suggestions available, so we hit a
DCHECK when the user opts in and we try to show them suggestions.

TBR=sebsg@chromium.org

Bug:  914944 
Change-Id: I38061b59699de623d0d828d1932dbfd1ad7aceee
Reviewed-on: https://chromium-review.googlesource.com/c/1376813
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Tommy Martino <tmartino@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#616459}(cherry picked from commit 3b5fea2584edb0bb8b155bed9cc386aa3f6c66c6)
Reviewed-on: https://chromium-review.googlesource.com/c/1380574
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#396}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/71cadb33ce6466480b127b7b65bf1e9a27bbc506/components/autofill/core/browser/autofill_manager.cc

Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/71cadb33ce6466480b127b7b65bf1e9a27bbc506

Commit: 71cadb33ce6466480b127b7b65bf1e9a27bbc506
Author: tmartino@chromium.org
Commiter: tmartino@chromium.org
Date: 2018-12-17 18:54:23 +0000 UTC

[Merge 72][Autofill Sync] Do not show downstream prompt on CVC fields

This CL fixes a small bug where the opt-in prompt for Autofill Sync
Transport is shown on CVC and CC type fields. This is necessary because
these fields don't actually have suggestions available, so we hit a
DCHECK when the user opts in and we try to show them suggestions.

TBR=sebsg@chromium.org

Bug:  914944 
Change-Id: I38061b59699de623d0d828d1932dbfd1ad7aceee
Reviewed-on: https://chromium-review.googlesource.com/c/1376813
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Tommy Martino <tmartino@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#616459}(cherry picked from commit 3b5fea2584edb0bb8b155bed9cc386aa3f6c66c6)
Reviewed-on: https://chromium-review.googlesource.com/c/1380574
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#396}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Sign in to add a comment