[BNPL] Add header and footer for BNPL ToS bottomsheet [chromium/src : main]

[Yishui Liu (Gerrit)]

Attention needed from Mohamed Amir Yosef, Timofey Chudakov, Vinny Persky and Viplav Kadam

Yishui Liu added 1 comment

Patchset-level comments

File-level comment, Patchset 34 (Latest):

Yishui Liu . resolved

Code reviews were reset accidentally by an upstream update. Cleaned unrelated change, and only minor change (TODO comment and spacing) since the latest review (Patchset 30).

Open in Gerrit

Related details

Attention is currently required from:

• Mohamed Amir Yosef
• Timofey Chudakov
• Vinny Persky
• Viplav Kadam

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I3e0281643020f716165f3519743a23e1b03854f7

Gerrit-Change-Number: 6985221

Gerrit-PatchSet: 34

Gerrit-Owner: Yishui Liu <yis...@google.com>

Gerrit-Reviewer: Mohamed Amir Yosef <ma...@chromium.org>

Gerrit-Reviewer: Timofey Chudakov <tchu...@google.com>

Gerrit-Reviewer: Vinny Persky <vinny...@google.com>

Gerrit-Reviewer: Viplav Kadam <vipla...@google.com>

Gerrit-Reviewer: Yishui Liu <yis...@google.com>

Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>

Gerrit-CC: Akihiro Ota <akihi...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Enterprise Policy Reviews <enterprise-p...@google.com>

Gerrit-CC: Javier Fernandez <jfern...@igalia.com>

Gerrit-CC: Peter Williamson <pet...@chromium.org>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Vinny Persky <vinny...@google.com>

Gerrit-Attention: Mohamed Amir Yosef <ma...@chromium.org>

Gerrit-Attention: Timofey Chudakov <tchu...@google.com>

Gerrit-Attention: Viplav Kadam <vipla...@google.com>

Gerrit-Comment-Date: Wed, 15 Oct 2025 00:00:53 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Yishui Liu (Gerrit)]

Attention needed from Mohamed Amir Yosef, Timofey Chudakov, Vinny Persky and Viplav Kadam

Yishui Liu added 1 comment

Patchset-level comments

File-level comment, Patchset 31:

Yishui Liu . resolved

Hi Mohamed,

Can you help review the change under chrome/browser/touch_to_fill/common/android/ and chrome/browser/touch_to_fill/password_manager/android/internal/java/src/org/chromium/chrome/browser/touch_to_fill/ TouchToFillView.java
for adding an additional footer type for the touch to fill view?

Thanks

Yishui Liu

And a common layout for text buttons.

Gerrit-Comment-Date: Wed, 15 Oct 2025 00:07:05 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Yishui Liu <yis...@google.com>

[Mohamed Amir Yosef (Gerrit)]

Attention needed from Timofey Chudakov, Vinny Persky, Viplav Kadam and Yishui Liu

Mohamed Amir Yosef voted and added 1 comment

Votes added by Mohamed Amir Yosef

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 34 (Latest):

Mohamed Amir Yosef . resolved

LGTM

Thank you!

Open in Gerrit

Related details

Attention is currently required from:

• Timofey Chudakov
• Vinny Persky
• Viplav Kadam

• Yishui Liu

Submit Requirements:

• Code-Coverage
• Code-Owners

• Code-Review
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I3e0281643020f716165f3519743a23e1b03854f7

Gerrit-Change-Number: 6985221

Gerrit-PatchSet: 34

Gerrit-Owner: Yishui Liu <yis...@google.com>

Gerrit-Reviewer: Mohamed Amir Yosef <ma...@chromium.org>

Gerrit-Reviewer: Timofey Chudakov <tchu...@google.com>

Gerrit-Reviewer: Vinny Persky <vinny...@google.com>

Gerrit-Reviewer: Viplav Kadam <vipla...@google.com>

Gerrit-Reviewer: Yishui Liu <yis...@google.com>

Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>

Gerrit-CC: Akihiro Ota <akihi...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Enterprise Policy Reviews <enterprise-p...@google.com>

Gerrit-CC: Javier Fernandez <jfern...@igalia.com>

Gerrit-CC: Peter Williamson <pet...@chromium.org>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Vinny Persky <vinny...@google.com>

Gerrit-Attention: Yishui Liu <yis...@google.com>

Gerrit-Attention: Timofey Chudakov <tchu...@google.com>

Gerrit-Attention: Viplav Kadam <vipla...@google.com>

Gerrit-Comment-Date: Wed, 15 Oct 2025 10:45:43 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Timofey Chudakov (Gerrit)]

Attention needed from Vinny Persky, Viplav Kadam and Yishui Liu

Timofey Chudakov added 15 comments

Commit Message

Line 7, Patchset 34 (Latest):[BNPL] Add header and footer for BNPL ToS bottomsheet

Timofey Chudakov . unresolved

Technically this could be 2 CLs

File chrome/android/java/src/org/chromium/chrome/browser/touch_to_fill/payments/TouchToFillPaymentMethodViewBridge.java

Line 183, Patchset 34 (Latest): @CalledByNative
private BnplIssuerTosDetail.LegalMessages convertLegalMessageLinesForBnplTos(
@JniType("std::vector") List<LegalMessageLine> legalMessageLines) {
return new BnplIssuerTosDetail.LegalMessages(legalMessageLines, this::openLink);
}

Timofey Chudakov . unresolved

Can you annotate the constructor of the `LegalMessages` with `@CalledByNative` so that you don't need to create an additional static method here?

File chrome/browser/touch_to_fill/autofill/android/internal/java/res/layout/touch_to_fill_legal_message_item.xml

Line 16, Patchset 34 (Latest): android:focusable="true"

Timofey Chudakov . unresolved

Why do you need to make this TextView focusable?

File chrome/browser/touch_to_fill/autofill/android/internal/java/src/org/chromium/chrome/browser/touch_to_fill/payments/TouchToFillPaymentMethodViewBinder.java

Line 763, Patchset 34 (Latest): private static void setCallbackForButton(View view, int buttonId, Runnable callback) {

Timofey Chudakov . unresolved

@IdRes int

File chrome/browser/touch_to_fill/autofill/android/internal/java/src/org/chromium/chrome/browser/touch_to_fill/payments/TouchToFillPaymentMethodViewTest.java

Line 1422, Patchset 34 (Latest): .perform(createClickActionWithFlags(MotionEvent.FLAG_WINDOW_IS_OBSCURED));

Timofey Chudakov . unresolved

If you just want to click a view, use `ViewActions::click()`. This one tests how the view handles clicks through obscured surfaces.

Line 1426, Patchset 34 (Latest): .perform(createClickActionWithFlags(MotionEvent.FLAG_WINDOW_IS_OBSCURED));

Timofey Chudakov . unresolved

Same here.

File chrome/browser/touch_to_fill/common/android/java/res/layout/touch_to_fill_text_button.xml

Line 10, Patchset 34 (Latest): android:descendantFocusability="blocksDescendants"

Timofey Chudakov . unresolved

I believe this has no effect when set on a View which is not a ViewGroup, please remove.

File components/autofill/android/java/src/org/chromium/components/autofill/payments/BnplIssuerTosDetail.java

Line 34, Patchset 34 (Latest): public LegalMessages(List<LegalMessageLine> lines, Consumer<String> linkOpener) {

Timofey Chudakov . unresolved

Can you annotate the constructor with @CalledByNative. It might be problematic to use it on the native side because the class is nested.

Line 41, Patchset 34 (Latest): private final int mHeaderIconDrawableId;

Timofey Chudakov . unresolved

@DrawableRes

Line 44, Patchset 34 (Latest): private final int mHeaderIconDarkDrawableId;

Timofey Chudakov . unresolved

@DrawableRes

Line 74, Patchset 34 (Latest): int headerIconDrawableId,

Timofey Chudakov . unresolved

@DrawableRes

Line 75, Patchset 34 (Latest): int headerIconDarkDrawableId,

Timofey Chudakov . unresolved

@DrawableRes

Line 90, Patchset 34 (Latest): public int getHeaderIconDrawableId() {

Timofey Chudakov . unresolved

@DrawableRes int

Line 94, Patchset 34 (Latest): public int getHeaderIconDarkDrawableId() {

Timofey Chudakov . unresolved

@DrawableRes int

File components/autofill/android/javatests/src/org/chromium/components/autofill/payments/BnplIssuerTosDetailTest.java

Line 29, Patchset 34 (Latest): /* headerIconDrawableId= */ 1,
/* headerIconDarkDrawableId= */ 2,

Timofey Chudakov . unresolved

Maybe use some general purpose icon ids here?

Open in Gerrit

Related details

Attention is currently required from:

• Vinny Persky
• Viplav Kadam
• Yishui Liu

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review

• No-Unresolved-Comments
• Review-Enforcement

Gerrit-Attention: Viplav Kadam <vipla...@google.com>

Gerrit-Comment-Date: Wed, 15 Oct 2025 12:27:43 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Vinny Persky (Gerrit)]

Attention needed from Viplav Kadam and Yishui Liu

Vinny Persky added 1 comment

Patchset-level comments

File-level comment, Patchset 34 (Latest):

Vinny Persky . resolved

Taking myself off until timofey's comments are resolved

Open in Gerrit

Related details

Attention is currently required from:

• Viplav Kadam
• Yishui Liu

Gerrit-Attention: Yishui Liu <yis...@google.com>

Gerrit-Attention: Viplav Kadam <vipla...@google.com>

Gerrit-Comment-Date: Wed, 15 Oct 2025 17:31:24 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Viplav Kadam (Gerrit)]

Attention needed from Yishui Liu

Viplav Kadam added 1 comment

File chrome/browser/touch_to_fill/autofill/android/internal/java/res/layout/touch_to_fill_legal_message_item.xml

Line 16, Patchset 34: android:focusable="true"

Timofey Chudakov . unresolved

Why do you need to make this TextView focusable?

Viplav Kadam

It's for accessibility. TalkBack will read the text, but it may skip over the it as an interactive element. Correct me if I am wrong.

Open in Gerrit

Related details

Attention is currently required from:

• Yishui Liu

Submit Requirements:

• Code-Coverage
• Code-Owners

• Code-Review

• No-Unresolved-Comments

• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I3e0281643020f716165f3519743a23e1b03854f7

Gerrit-Change-Number: 6985221

Gerrit-PatchSet: 36

Gerrit-Owner: Yishui Liu <yis...@google.com>

Gerrit-Reviewer: Mohamed Amir Yosef <ma...@chromium.org>

Gerrit-Reviewer: Timofey Chudakov <tchu...@google.com>

Gerrit-Reviewer: Vinny Persky <vinny...@google.com>

Gerrit-Reviewer: Viplav Kadam <vipla...@google.com>

Gerrit-Reviewer: Yishui Liu <yis...@google.com>

Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>

Gerrit-CC: Akihiro Ota <akihi...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Enterprise Policy Reviews <enterprise-p...@google.com>

Gerrit-CC: Javier Fernandez <jfern...@igalia.com>

Gerrit-CC: Peter Williamson <pet...@chromium.org>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Yishui Liu <yis...@google.com>

Gerrit-Comment-Date: Wed, 15 Oct 2025 22:31:18 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Timofey Chudakov <tchu...@google.com>