Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(33)

Issue 2698703003: [Payments] Add UI elements to secure branding for payments (Closed)

Created:
3 years, 10 months ago by gogerald1
Modified:
3 years, 10 months ago
CC:
chromium-reviews, rouslan+payments_chromium.org, sebsg+paymentswatch_chromium.org, gogerald+paymentswatch_chromium.org, agrieve+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add UI elements to secure branding for payments BUG=691104 Review-Url: https://codereview.chromium.org/2698703003 Cr-Commit-Position: refs/heads/master@{#451873} Committed: https://chromium.googlesource.com/chromium/src/+/41f07f2348e5ccec788938057ff29fea5876ff57

Patch Set 1 #

Total comments: 10

Patch Set 2 : Address comments, fix animation and improve efficiency #

Total comments: 14

Patch Set 3 : address comments and fit small device dynamically #

Total comments: 11

Patch Set 4 : format #

Patch Set 5 : fix tests #

Patch Set 6 : Measure bottom bar manually #

Patch Set 7 : support rotation #

Total comments: 10

Patch Set 8 : rename #

Patch Set 9 : address comments #

Total comments: 4

Patch Set 10 : nits #

Messages

Total messages: 79 (53 generated)
gogerald1
Hi dfalcantara and rouslan@, PTAL, please refer the screenshots in the bug.
3 years, 10 months ago (2017-02-15 21:37:50 UTC) #10
please use gerrit instead
+palmer@: Would you be the right person to review security enamel related changes? https://codereview.chromium.org/2698703003/diff/60001/components/url_formatter/android/java/src/org/chromium/components/url_formatter/UrlFormatter.java File ...
3 years, 10 months ago (2017-02-15 21:45:03 UTC) #12
please use gerrit instead
https://codereview.chromium.org/2698703003/diff/60001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java File chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java (right): https://codereview.chromium.org/2698703003/diff/60001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java#newcode552 chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:552: ApiCompatibilityUtils.setCompoundDrawablesRelativeWithIntrinsicBounds(hostName, Also, let's not show the lock icon for ...
3 years, 10 months ago (2017-02-15 21:54:46 UTC) #13
palmer
> +palmer@: Would you be the right person to review security enamel related changes? Not ...
3 years, 10 months ago (2017-02-16 02:00:08 UTC) #16
please use gerrit instead
3 years, 10 months ago (2017-02-16 14:31:48 UTC) #18
gogerald1
Hi rouslan@, PTAL https://codereview.chromium.org/2698703003/diff/60001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java File chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java (right): https://codereview.chromium.org/2698703003/diff/60001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java#newcode552 chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:552: ApiCompatibilityUtils.setCompoundDrawablesRelativeWithIntrinsicBounds(hostName, On 2017/02/15 21:54:46, rouslan wrote: ...
3 years, 10 months ago (2017-02-16 16:43:14 UTC) #21
gone
https://codereview.chromium.org/2698703003/diff/120001/chrome/android/java/res/layout/payment_request.xml File chrome/android/java/res/layout/payment_request.xml (right): https://codereview.chromium.org/2698703003/diff/120001/chrome/android/java/res/layout/payment_request.xml#newcode35 chrome/android/java/res/layout/payment_request.xml:35: <LinearLayout Doing this means that the buttons will never ...
3 years, 10 months ago (2017-02-16 19:18:57 UTC) #22
please use gerrit instead
https://codereview.chromium.org/2698703003/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java File chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java (right): https://codereview.chromium.org/2698703003/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java#newcode550 chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:550: if (origin.startsWith(UrlConstants.HTTPS_SCHEME)) { Let's use HTTPS_URL_PREFIX instead of HTTPS_SCHEME ...
3 years, 10 months ago (2017-02-16 19:41:51 UTC) #23
gogerald1
Hi dfalcantara@ and rouslan@, please take another look, https://codereview.chromium.org/2698703003/diff/120001/chrome/android/java/res/layout/payment_request.xml File chrome/android/java/res/layout/payment_request.xml (right): https://codereview.chromium.org/2698703003/diff/120001/chrome/android/java/res/layout/payment_request.xml#newcode35 chrome/android/java/res/layout/payment_request.xml:35: <LinearLayout ...
3 years, 10 months ago (2017-02-17 00:44:46 UTC) #27
gone
https://codereview.chromium.org/2698703003/diff/180001/chrome/android/java/res/layout/payment_request_bottom_bar.xml File chrome/android/java/res/layout/payment_request_bottom_bar.xml (right): https://codereview.chromium.org/2698703003/diff/180001/chrome/android/java/res/layout/payment_request_bottom_bar.xml#newcode17 chrome/android/java/res/layout/payment_request_bottom_bar.xml:17: <ImageView Indent by 4s https://codereview.chromium.org/2698703003/diff/180001/chrome/android/java/res/layout/payment_request_header.xml File chrome/android/java/res/layout/payment_request_header.xml (right): https://codereview.chromium.org/2698703003/diff/180001/chrome/android/java/res/layout/payment_request_header.xml#newcode59 ...
3 years, 10 months ago (2017-02-17 01:53:54 UTC) #29
gogerald1
Hi dfalcantara@, PTAL, https://codereview.chromium.org/2698703003/diff/180001/chrome/android/java/res/layout/payment_request_bottom_bar.xml File chrome/android/java/res/layout/payment_request_bottom_bar.xml (right): https://codereview.chromium.org/2698703003/diff/180001/chrome/android/java/res/layout/payment_request_bottom_bar.xml#newcode17 chrome/android/java/res/layout/payment_request_bottom_bar.xml:17: <ImageView On 2017/02/17 01:53:54, dfalcantara (load ...
3 years, 10 months ago (2017-02-17 17:28:29 UTC) #37
gone
https://codereview.chromium.org/2698703003/diff/180001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestBottomBar.java File chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestBottomBar.java (right): https://codereview.chromium.org/2698703003/diff/180001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestBottomBar.java#newcode31 chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestBottomBar.java:31: findViewById(R.id.logo).setVisibility(View.VISIBLE); On 2017/02/17 17:28:28, gogerald1 wrote: > On 2017/02/17 ...
3 years, 10 months ago (2017-02-17 19:02:55 UTC) #40
gogerald1
https://codereview.chromium.org/2698703003/diff/180001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestBottomBar.java File chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestBottomBar.java (right): https://codereview.chromium.org/2698703003/diff/180001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestBottomBar.java#newcode31 chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestBottomBar.java:31: findViewById(R.id.logo).setVisibility(View.VISIBLE); On 2017/02/17 19:02:55, dfalcantara (load balance plz) wrote: ...
3 years, 10 months ago (2017-02-17 22:05:39 UTC) #42
gogerald1
Adds the support of rotation, ptal,
3 years, 10 months ago (2017-02-17 22:31:42 UTC) #44
gogerald1
corrected the variable names, please review the patch 8,
3 years, 10 months ago (2017-02-17 23:10:17 UTC) #47
gone
https://codereview.chromium.org/2698703003/diff/280001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestBottomBar.java File chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestBottomBar.java (right): https://codereview.chromium.org/2698703003/diff/280001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestBottomBar.java#newcode26 chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestBottomBar.java:26: View logoWithName = findViewById(R.id.logo_name); Cache the views in onFinishInflate() ...
3 years, 10 months ago (2017-02-17 23:17:11 UTC) #48
gogerald1
Thanks, ptal, https://codereview.chromium.org/2698703003/diff/280001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestBottomBar.java File chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestBottomBar.java (right): https://codereview.chromium.org/2698703003/diff/280001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestBottomBar.java#newcode26 chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestBottomBar.java:26: View logoWithName = findViewById(R.id.logo_name); On 2017/02/17 23:17:10, ...
3 years, 10 months ago (2017-02-20 15:18:36 UTC) #58
gone
lgtm
3 years, 10 months ago (2017-02-21 18:27:25 UTC) #63
please use gerrit instead
LGTM % nits By the way, the elide is green: https://drive.google.com/open?id=1nbDbfsKJSSipZ0v-lFSACS4LDJCSgCfleyPx3WD9kQo Is that fixable? Not ...
3 years, 10 months ago (2017-02-21 19:51:08 UTC) #64
gogerald1
Thanks, created a lower priority bug (https://bugs.chromium.org/p/chromium/issues/detail?id=693615) for the 'elide is green' as mentioned. https://codereview.chromium.org/2698703003/diff/340001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestBottomBar.java ...
3 years, 10 months ago (2017-02-21 20:11:51 UTC) #65
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2698703003/360001
3 years, 10 months ago (2017-02-21 20:13:18 UTC) #68
commit-bot: I haz the power
Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_daisy_chromium_compile_only_ng on ...
3 years, 10 months ago (2017-02-21 22:16:22 UTC) #70
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2698703003/360001
3 years, 10 months ago (2017-02-21 22:25:36 UTC) #72
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
3 years, 10 months ago (2017-02-22 00:30:32 UTC) #74
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2698703003/360001
3 years, 10 months ago (2017-02-22 00:45:52 UTC) #76
commit-bot: I haz the power
3 years, 10 months ago (2017-02-22 01:29:27 UTC) #79
Message was sent while issue was closed.
Committed patchset #10 (id:360001) as
https://chromium.googlesource.com/chromium/src/+/41f07f2348e5ccec788938057ff2...

Powered by Google App Engine
This is Rietveld 408576698