New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 782663 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 0
Type: Bug

Blocking:
issue 753806



Sign in to add a comment

[Password Manager] Show confirmation bubble as manual fallback for generated cases

Project Member Reported by kolos@chromium.org, Nov 8 2017

Issue description

1. Generate a password on a form
2. Click the omnibox icon.

What is expected behavior? 
Confirmation bubble should pop up.

What happened instead?
A save bubble pops up. It can generate duplicate entries in the store.
 
Nov-07-2017 13-35-46.gif
26.9 MB Download

Comment 1 by kolos@chromium.org, Nov 8 2017

Description: Show this description

Comment 2 by kolos@chromium.org, Nov 8 2017

Labels: ReleaseBlock-Stable M-63
This is requirement to merge it to M-63.
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 9 2017

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

commit eb2186f3682000349de07278cc013da765ba364c
Author: Maxim Kolosovskiy <kolos@chromium.org>
Date: Thu Nov 09 08:27:49 2017

[Password Manager] Show confirmation bubble as manual fallback for generated cases

Bug:  782663 ,  725883 
Change-Id: I1e836ff22ab8e729a5b2a30519fc8e9b7579e151
Reviewed-on: https://chromium-review.googlesource.com/758270
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515123}
[modify] https://crrev.com/eb2186f3682000349de07278cc013da765ba364c/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc
[modify] https://crrev.com/eb2186f3682000349de07278cc013da765ba364c/chrome/renderer/autofill/fake_content_password_manager_driver.cc
[modify] https://crrev.com/eb2186f3682000349de07278cc013da765ba364c/chrome/renderer/autofill/fake_content_password_manager_driver.h
[modify] https://crrev.com/eb2186f3682000349de07278cc013da765ba364c/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/eb2186f3682000349de07278cc013da765ba364c/chrome/renderer/autofill/password_generation_agent_browsertest.cc
[modify] https://crrev.com/eb2186f3682000349de07278cc013da765ba364c/components/autofill/content/renderer/password_generation_agent.cc
[modify] https://crrev.com/eb2186f3682000349de07278cc013da765ba364c/components/password_manager/core/browser/password_form_metrics_recorder.cc
[modify] https://crrev.com/eb2186f3682000349de07278cc013da765ba364c/components/password_manager/core/browser/password_manager_metrics_util.h

 kolos can you please apply "Merge-Request-63" label for Release team review and approve the CL to get merged to M63.
Cc: gov...@chromium.org

Comment 6 by kolos@chromium.org, Nov 9 2017

Labels: Merge-Request-63
UI requested to merge it back to M-63. Otherwise, UI would be confusing in some cases. 

govind@: I will wait till it is verified in Canary. 
Thank you kolos@.
Could you please us know why multiple merges are coming for Password Manager? Are they M63 regressions or fixes for new feature?

There is one more merge besides this one - https://bugs.chromium.org/p/chromium/issues/detail?id=782164#c5.

Project Member

Comment 8 by sheriffbot@chromium.org, Nov 10 2017

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

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: ranjitkan@chromium.org
Labels: TE-Verified-M64 TE-Verified-64.0.3264.0
Rechecked this issue on Windows 10, Mac 10.12.6, Ubuntu 14.04 using chrome version 64.0.3264.0 and fix is working as intended. Followed the steps mentioned in the issue description and now confirmation bubble is displayed. Screen shot attached.

Tagging the issue with TE-verified labels for M64.

Thanks.!
Confirmation Bubble.png
1.3 MB View Download

Comment 10 by kolos@chromium.org, Nov 10 2017

Cc: melandory@chromium.org battre@chromium.org
To #7: it is new feature (Issue 768964). It is just bad luck that we didn't detect these bugs before. Sorry. 
Labels: -Merge-Review-63 Merge-Approved-63
Approving merge to M63 branch 3239 based on comment #9 and #10. Please merge ASAP so we can pick it up for next week beta release. Thank you.
Project Member

Comment 12 by bugdroid1@chromium.org, Nov 11 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/06c9d3bc08a493a1fef3ff7879b3f9fcfde3e517

commit 06c9d3bc08a493a1fef3ff7879b3f9fcfde3e517
Author: Maxim Kolosovskiy <kolos@chromium.org>
Date: Sat Nov 11 10:15:14 2017

[Password Manager] Show confirmation bubble as manual fallback for generated cases

Bug:  782663 ,  725883 
Change-Id: I1e836ff22ab8e729a5b2a30519fc8e9b7579e151
Reviewed-on: https://chromium-review.googlesource.com/758270
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#515123}(cherry picked from commit eb2186f3682000349de07278cc013da765ba364c)
Reviewed-on: https://chromium-review.googlesource.com/765327
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#452}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/06c9d3bc08a493a1fef3ff7879b3f9fcfde3e517/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc
[modify] https://crrev.com/06c9d3bc08a493a1fef3ff7879b3f9fcfde3e517/chrome/renderer/autofill/fake_content_password_manager_driver.cc
[modify] https://crrev.com/06c9d3bc08a493a1fef3ff7879b3f9fcfde3e517/chrome/renderer/autofill/fake_content_password_manager_driver.h
[modify] https://crrev.com/06c9d3bc08a493a1fef3ff7879b3f9fcfde3e517/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/06c9d3bc08a493a1fef3ff7879b3f9fcfde3e517/chrome/renderer/autofill/password_generation_agent_browsertest.cc
[modify] https://crrev.com/06c9d3bc08a493a1fef3ff7879b3f9fcfde3e517/components/autofill/content/renderer/password_generation_agent.cc
[modify] https://crrev.com/06c9d3bc08a493a1fef3ff7879b3f9fcfde3e517/components/password_manager/core/browser/password_form_metrics_recorder.cc
[modify] https://crrev.com/06c9d3bc08a493a1fef3ff7879b3f9fcfde3e517/components/password_manager/core/browser/password_manager_metrics_util.h

M63 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge  into the release branch ASAP. Thank you.



Comment 14 by kolos@chromium.org, Nov 13 2017

Status: Fixed (was: Started)

Sign in to add a comment