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

Issue 831751 link

Starred by 2 users
Status: Verified
Owner:
█ asimjour@chromium.org
Last visit > 30 days ago
Closed: May 2018
Cc:
█ asimjour@chromium.org
dbbrooks@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug
Proj-XR
Proj-XR-VR



Sign in to add a comment

Autofill - CVC dialog presented outside VR while entering Credit card

Project Member Reported by dougman@chromium.org, Apr 11 2018 
Chrome Version: (copy from chrome://version)
OS: (e.g. Win10, MacOS 10.12, etc...)

What steps will reproduce the problem?
(1) Navigate to https://rsolomakhin.github.io/autofill/
(2) Enter VR Browsing
(3) Use autofill to fill in the "Credit Card autofill" form, need a real credit card I think.

What is the expected result?
User should be presented with the CVC dialog in VR.

What happens instead?
User is presented with the CVC dialog but it is rendered outside VR.

I have not been able to get this dialog without a real credit card.
CVC dialog outside VR.png
92.6 KB View Download

Comment 1 by asimjour@chromium.org, Apr 11 2018

Cc: asimjour@chromium.org

Comment 2 by sko...@chromium.org, Apr 13 2018

Labels: -Pri-2 Pri-1
Owner: asimjour@chromium.org
Status: Assigned (was: Untriaged)

Comment 3 by ma...@chromium.org, May 1 2018

Components: UI>Browser>Autofill
Project Member

Comment 4 by bugdroid1@chromium.org, May 7 2018 

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

commit d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b
Author: Amirhossein Simjour <asimjour@chromium.org>
Date: Mon May 07 15:27:21 2018

Stop CVC popup in VR

When in VR, CVC popup does not show properly, so there is no support
for server side credit cards.
GetCreditCardSuggestions in autofill_manager is updated to support a flag
to decide whether the server side cards should be included or not.

When in VR, we set the flag (include_server_cards) to false and don't show
suggestions for server side cards.

Bug:  831751 

Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I86b6a87ba88109d01ef84a6622096118f9970486
Reviewed-on: https://chromium-review.googlesource.com/1031200
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Amirhossein Simjour <asimjour@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556446}
[modify] https://crrev.com/d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b/android_webview/browser/aw_autofill_client.cc
[modify] https://crrev.com/d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b/android_webview/browser/aw_autofill_client.h
[modify] https://crrev.com/d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b/chrome/browser/autofill/android/personal_data_manager_android.cc
[modify] https://crrev.com/d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b/chrome/browser/ui/autofill/chrome_autofill_client.cc
[modify] https://crrev.com/d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b/chrome/browser/ui/autofill/chrome_autofill_client.h
[modify] https://crrev.com/d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b/components/autofill/core/browser/autofill_client.h
[modify] https://crrev.com/d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b/components/autofill/core/browser/autofill_manager.cc
[modify] https://crrev.com/d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b/components/autofill/core/browser/personal_data_manager.cc
[modify] https://crrev.com/d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b/components/autofill/core/browser/personal_data_manager.h
[modify] https://crrev.com/d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b/components/autofill/core/browser/personal_data_manager_unittest.cc
[modify] https://crrev.com/d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b/components/autofill/core/browser/test_autofill_client.cc
[modify] https://crrev.com/d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b/components/autofill/core/browser/test_autofill_client.h
[modify] https://crrev.com/d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b/components/payments/content/payment_request_state.cc
[modify] https://crrev.com/d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b/ios/chrome/browser/payments/payment_request.mm
[modify] https://crrev.com/d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b/ios/chrome/browser/ui/autofill/chrome_autofill_client_ios.h
[modify] https://crrev.com/d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b/ios/chrome/browser/ui/autofill/chrome_autofill_client_ios.mm
[modify] https://crrev.com/d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b/ios/web_view/internal/autofill/web_view_autofill_client_ios.h
[modify] https://crrev.com/d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b/ios/web_view/internal/autofill/web_view_autofill_client_ios.mm

Comment 5 by asimjour@chromium.org, May 9 2018

Labels: Merge-Request-67
The change is verified on Canary.
Project Member

Comment 6 by sheriffbot@chromium.org, May 9 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: M67 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), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 7 by cmasso@google.com, May 9 2018 

I don't feel comfortable merging this change into M67. Can you elaborate more on why this has to be merged? How are users going to be affected if it is not merged
? what is the overall risk associated with merging this cl?

Comment 8 by asimjour@chromium.org, May 10 2018 

The change only has effect when user is in VR browsing mode. It touches many files just to add the new argument to the method call, but the actual code is only in 2 files, personal_data_manager.cc and chrome_autofill_client.cc. A new test is added to personal_data_manager_unittest.cc, so I don't think that there is any risk involved.

Comment 9 by asimjour@chromium.org, May 10 2018 

Without this patch the user will see the list of credit cards, but selecting an item results to breaking VR experience. A super bright 2D popup will open on top of VR, and user will be forced to take off the headset.

Comment 10 by cma...@chromium.org, May 10 2018

Labels: -Hotlist-Merge-Review -Merge-Review-67 Merge-Approved-67
Project Member

Comment 11 by bugdroid1@chromium.org, May 10 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/50f8f8228b9656aaddb98270d0560230d930933f

commit 50f8f8228b9656aaddb98270d0560230d930933f
Author: Amirhossein Simjour <asimjour@chromium.org>
Date: Thu May 10 20:53:35 2018

Stop CVC popup in VR

When in VR, CVC popup does not show properly, so there is no support
for server side credit cards.
GetCreditCardSuggestions in autofill_manager is updated to support a flag
to decide whether the server side cards should be included or not.

When in VR, we set the flag (include_server_cards) to false and don't show
suggestions for server side cards.

Bug:  831751 

Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I86b6a87ba88109d01ef84a6622096118f9970486
Reviewed-on: https://chromium-review.googlesource.com/1031200
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Amirhossein Simjour <asimjour@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#556446}(cherry picked from commit d3b7ba4a4b1b3fc51f8fb60c545b386e3912c57b)
Reviewed-on: https://chromium-review.googlesource.com/1054203
Reviewed-by: Amirhossein Simjour <asimjour@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#558}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/50f8f8228b9656aaddb98270d0560230d930933f/android_webview/browser/aw_autofill_client.cc
[modify] https://crrev.com/50f8f8228b9656aaddb98270d0560230d930933f/android_webview/browser/aw_autofill_client.h
[modify] https://crrev.com/50f8f8228b9656aaddb98270d0560230d930933f/chrome/browser/autofill/android/personal_data_manager_android.cc
[modify] https://crrev.com/50f8f8228b9656aaddb98270d0560230d930933f/chrome/browser/ui/autofill/chrome_autofill_client.cc
[modify] https://crrev.com/50f8f8228b9656aaddb98270d0560230d930933f/chrome/browser/ui/autofill/chrome_autofill_client.h
[modify] https://crrev.com/50f8f8228b9656aaddb98270d0560230d930933f/components/autofill/core/browser/autofill_client.h
[modify] https://crrev.com/50f8f8228b9656aaddb98270d0560230d930933f/components/autofill/core/browser/autofill_manager.cc
[modify] https://crrev.com/50f8f8228b9656aaddb98270d0560230d930933f/components/autofill/core/browser/personal_data_manager.cc
[modify] https://crrev.com/50f8f8228b9656aaddb98270d0560230d930933f/components/autofill/core/browser/personal_data_manager.h
[modify] https://crrev.com/50f8f8228b9656aaddb98270d0560230d930933f/components/autofill/core/browser/personal_data_manager_unittest.cc
[modify] https://crrev.com/50f8f8228b9656aaddb98270d0560230d930933f/components/autofill/core/browser/test_autofill_client.cc
[modify] https://crrev.com/50f8f8228b9656aaddb98270d0560230d930933f/components/autofill/core/browser/test_autofill_client.h
[modify] https://crrev.com/50f8f8228b9656aaddb98270d0560230d930933f/components/payments/content/payment_request_state.cc
[modify] https://crrev.com/50f8f8228b9656aaddb98270d0560230d930933f/ios/chrome/browser/payments/payment_request.mm
[modify] https://crrev.com/50f8f8228b9656aaddb98270d0560230d930933f/ios/chrome/browser/ui/autofill/chrome_autofill_client_ios.h
[modify] https://crrev.com/50f8f8228b9656aaddb98270d0560230d930933f/ios/chrome/browser/ui/autofill/chrome_autofill_client_ios.mm
[modify] https://crrev.com/50f8f8228b9656aaddb98270d0560230d930933f/ios/web_view/internal/autofill/web_view_autofill_client_ios.h
[modify] https://crrev.com/50f8f8228b9656aaddb98270d0560230d930933f/ios/web_view/internal/autofill/web_view_autofill_client_ios.mm

Comment 12 by asimjour@chromium.org, May 10 2018

Status: Fixed (was: Assigned)

Comment 13 by dougman@chromium.org, May 14 2018

Labels: Test-Complete
Status: Verified (was: Fixed)
Verified in build 67.0.3396.46 Beta.  The fix is in build 67.0.3396.43 which hasn't been released to beta.  build .46+ is planned to be released to beta this week.  The fix looks good.  I no longer see credit card info.
►

Sign in to add a comment