Avi Drissman (Gerrit)

unread,

Mar 12, 2026, 2:19:11 PM (12 days ago) Mar 12

to John An, Mirko Bonadei, Jerome Jiang, Avi Drissman, Jonathan Ross, Claire Chambers, AyeAye, Olga Gerchikov, Chromium LUCI CQ, Gaston Rodriguez, chromium...@chromium.org, devtools...@chromium.org, titoua...@chromium.org, ortuno...@chromium.org, cblume...@chromium.org, jz...@chromium.org, chrome-intell...@chromium.org, fgal...@chromium.org, mar...@chromium.org, penghuan...@chromium.org, chrome-intelligence-te...@google.com, roblia...@chromium.org, sky+...@chromium.org, net-r...@chromium.org, mac-r...@chromium.org

Attention needed from John An

Avi Drissman added 2 comments

File ui/base/test/ui_controls_aura.cc

Line 67, Patchset 16 (Latest): "implemented on Android, Mac and Linux.";

Avi Drissman . unresolved

The Mac doesn’t use Aura, so for accuracy you may want to remove it from this comment.

File ui/base/test/ui_controls_mac.mm

Line 331, Patchset 16 (Latest):

    NOTIMPLEMENTED_LOG_ONCE() << "Right Alt key test util support is not "
                                 "implemented on Android, Mac and Linux.";

Avi Drissman . unresolved

While Android and Linux do have a concept of AltGr, there is no such thing on the Mac. This is a Mac-only file, so consider only mentioning the Mac, with something like

```suggestion
    NOTIMPLEMENTED_LOG_ONCE() << "Right Alt testing is inapplicable on the Mac.";
```

Open in Gerrit

Related details

Attention is currently required from:

John An

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

satisfied_requirement

unsatisfied_requirement

open

diffy

John An (Gerrit)

unread,

Mar 12, 2026, 4:32:42 PM (12 days ago) Mar 12

to Mirko Bonadei, Jerome Jiang, Claire Chambers, AyeAye, Olga Gerchikov, Chromium LUCI CQ, Gaston Rodriguez, chromium...@chromium.org, devtools...@chromium.org, titoua...@chromium.org, ortuno...@chromium.org, cblume...@chromium.org, jz...@chromium.org, chrome-intell...@chromium.org, fgal...@chromium.org, mar...@chromium.org, penghuan...@chromium.org, chrome-intelligence-te...@google.com, roblia...@chromium.org, sky+...@chromium.org, net-r...@chromium.org, mac-r...@chromium.org

John An added 2 comments

Patchset-level comments

File-level comment, Patchset 16 (Latest):

John An . resolved

Removing Avi Drissman and Jon Ross until CL is ready. CQ issues will take some time to debug. Apologies for the noise!

File chrome/test/base/interactive_test_utils.h

Line 99, Patchset 10:

    bool control,
    bool shift,
    bool alt,
    bool command,
    ui_controls::KeyEventType wait_for = ui_controls::kKeyRelease,
    bool right_alt = false);

Avi Drissman . resolved

I’m somewhat concerned for the long term about a bunch of `bool`s in a row and the ease of mixing them up. Sometimes `right_alt` immediately follows `alt` but sometimes, like here, it’s at the end so that it can have a default value applied.
I’m wondering if, for a followup cleanup CL, we can abstract the notion of “bools for modifier keys” into a dedicated struct. Even something as simple as five `bools` (defaulted to `false`) would let us use designated initialization for clarity.

John An

I'm in favor of a bools for modifiers keys struct and can own that. Certainly would have made this change easier.

Open in Gerrit

Related details

Attention set is empty

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

satisfied_requirement

unsatisfied_requirement

open

diffy

John An (Gerrit)

unread,

Mar 23, 2026, 7:13:43 PM (10 hours ago) Mar 23

to Avi Drissman, James Su, Mirko Bonadei, Jerome Jiang, Claire Chambers, AyeAye, Olga Gerchikov, Chromium LUCI CQ, Gaston Rodriguez, chromium...@chromium.org, jbauma...@chromium.org, alexmo...@chromium.org, creis...@chromium.org, keithle...@chromium.org, navigation...@chromium.org, yhanad...@chromium.org, tranbaod...@chromium.org, nona+...@chromium.org, shuche...@chromium.org, devtools...@chromium.org, titoua...@chromium.org, ortuno...@chromium.org, cblume...@chromium.org, jz...@chromium.org, chrome-intell...@chromium.org, fgal...@chromium.org, mar...@chromium.org, penghuan...@chromium.org, chrome-intelligence-te...@google.com, roblia...@chromium.org, sky+...@chromium.org, net-r...@chromium.org, mac-r...@chromium.org

Attention needed from Avi Drissman

John An added 4 comments

Patchset-level comments

File-level comment, Patchset 24 (Latest):

John An . resolved

CQ is passing now and test code is refactored. PTAL!

File ui/base/test/ui_controls_aura.cc

Line 67, Patchset 16: "implemented on Android, Mac and Linux.";

Avi Drissman . resolved

The Mac doesn’t use Aura, so for accuracy you may want to remove it from this comment.

John An

Done

File ui/base/test/ui_controls_mac.mm

Line 330, Patchset 10:

  // This doesn't time out if `window` is deleted before the key release events
  // are dispatched, so it's fine to ignore `wait_for` and always wait for key
  // release events.

Avi Drissman . resolved

Move the comment to precede the code to which it applies.

John An

Done

Line 331, Patchset 16: NOTIMPLEMENTED_LOG_ONCE() << "Right Alt key test util support is not "


                                 "implemented on Android, Mac and Linux.";

Avi Drissman . resolved

While Android and Linux do have a concept of AltGr, there is no such thing on the Mac. This is a Mac-only file, so consider only mentioning the Mac, with something like
```suggestion
    NOTIMPLEMENTED_LOG_ONCE() << "Right Alt testing is inapplicable on the Mac.";
```

John An

Done

Open in Gerrit

Related details

Attention is currently required from:

Avi Drissman

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

satisfied_requirement

unsatisfied_requirement

open

diffy

gwsq (Gerrit)

unread,

Mar 23, 2026, 7:19:38 PM (10 hours ago) Mar 23

to John An, Chromium UI Views Reviews, Mike Wasserman, Avi Drissman, James Su, Mirko Bonadei, Jerome Jiang, Claire Chambers, AyeAye, Olga Gerchikov, Chromium LUCI CQ, Gaston Rodriguez, chromium...@chromium.org, jbauma...@chromium.org, alexmo...@chromium.org, creis...@chromium.org, keithle...@chromium.org, navigation...@chromium.org, yhanad...@chromium.org, tranbaod...@chromium.org, nona+...@chromium.org, shuche...@chromium.org, devtools...@chromium.org, titoua...@chromium.org, ortuno...@chromium.org, cblume...@chromium.org, jz...@chromium.org, chrome-intell...@chromium.org, fgal...@chromium.org, mar...@chromium.org, penghuan...@chromium.org, chrome-intelligence-te...@google.com, roblia...@chromium.org, sky+...@chromium.org, net-r...@chromium.org, mac-r...@chromium.org

Attention needed from Avi Drissman and Mike Wasserman

Message from gwsq

Reviewer source(s):
m...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ui/views/config.gwsq)

Open in Gerrit

Related details

Attention is currently required from:

Avi Drissman
Mike Wasserman

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

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

Interactive UI Tests for Arabic-Indic Digit Input on Windows [chromium/src : main]

Avi Drissman (Gerrit)

Avi Drissman added 2 comments

Related details

John An (Gerrit)

John An added 2 comments

Related details

John An (Gerrit)

John An added 4 comments

Related details

gwsq (Gerrit)

Message from gwsq

Related details