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

Issue 770303 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

canMakePayment behavior in incognito mode

Project Member Reported by rouslan@chromium.org, Sep 29 2017

Issue description

canMakePayment on desktop in incognito mode should return 'false' for payment app method names, because that feature is not available yet.
 
Labels: M-62
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 3 2017

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

commit 1804ee43e30d4873cdea6e560da1f812a75ce83b
Author: Rouslan Solomakhin <rouslan@chromium.org>
Date: Tue Oct 03 14:27:43 2017

[Payments] canMakePayment() in incognito mode.

Before this patch, a canMakePayment() check for payment apps returned
'true' on desktop and iOS in incognito mode, even though payment apps
are not supported on these platforms yet.

    new PaymentRequest('https://bobpay.com').canMakePayment()

The same canMakePayment() check in non-incognito mode returns 'false'.
To preserve user privacy better, the canMakePayment() check should
return 'false' in incognito mode as well.

This patch changes the return value from canMakePayment() in incognito
mode such that, if only payment apps are requested and the payment app
feature is not available, then canMakePayment() returns 'false'.

After this patch, the canMakePayment() check for payment apps on desktop
and iOS returns 'false'.

Bug:  770303 
Change-Id: I86e8089790fa83764eec0ae120e39559724a5118
Reviewed-on: https://chromium-review.googlesource.com/693079
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: anthonyvd <anthonyvd@chromium.org>
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506029}
[modify] https://crrev.com/1804ee43e30d4873cdea6e560da1f812a75ce83b/chrome/browser/ui/views/payments/payment_request_can_make_payment_browsertest.cc
[modify] https://crrev.com/1804ee43e30d4873cdea6e560da1f812a75ce83b/components/payments/content/can_make_payment_query_factory.cc
[modify] https://crrev.com/1804ee43e30d4873cdea6e560da1f812a75ce83b/components/payments/content/can_make_payment_query_factory.h
[modify] https://crrev.com/1804ee43e30d4873cdea6e560da1f812a75ce83b/components/payments/content/payment_request.cc
[modify] https://crrev.com/1804ee43e30d4873cdea6e560da1f812a75ce83b/components/payments/content/payment_request.h
[modify] https://crrev.com/1804ee43e30d4873cdea6e560da1f812a75ce83b/components/payments/content/payment_request_spec.cc
[modify] https://crrev.com/1804ee43e30d4873cdea6e560da1f812a75ce83b/components/payments/content/payment_request_spec.h
[modify] https://crrev.com/1804ee43e30d4873cdea6e560da1f812a75ce83b/components/test/data/payments/payment_method_identifier.js
[modify] https://crrev.com/1804ee43e30d4873cdea6e560da1f812a75ce83b/components/test/data/payments/payment_request_payment_method_identifier_test.html
[modify] https://crrev.com/1804ee43e30d4873cdea6e560da1f812a75ce83b/ios/chrome/browser/ui/payments/payment_request_manager.mm

Labels: Merge-Request-62
Verified in canary. Would like to merge to M-62, because this affects partners. The change has test coverage.
Project Member

Comment 4 by sheriffbot@chromium.org, Oct 4 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: We are only 12 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Is this truly a critical regression? Since we're only 2 weeks away from M62 Stable, would like to ensure we only take critical merges. 
Labels: -Hotlist-Merge-Review -Merge-Review-62
Status: Fixed (was: Started)
We've discussed this and decided to not merge. Thank you.

Sign in to add a comment