[Omnibox Next] Add context entrypoint and dropdown [chromium/src : main]
1 view
Skip to first unread message
Marlon Facey (Gerrit)
Apr 14, 2026, 5:02:54 PMApr 14
to Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, penghuan...@chromium.org, devtools...@chromium.org, jz...@chromium.org, mar...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org, fgal...@chromium.org, feature-me...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, oshima...@chromium.org
Marlon Facey voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Marlon Facey (Gerrit)
Apr 14, 2026, 5:04:54 PMApr 14
to Sho Mizoe, Bofeng Chen, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, penghuan...@chromium.org, devtools...@chromium.org, jz...@chromium.org, mar...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org, fgal...@chromium.org, feature-me...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, oshima...@chromium.org
Attention needed from Bofeng Chen and Sho Mizoe
Marlon Facey voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Bofeng Chen
- Sho Mizoe
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sho Mizoe (Gerrit)
Apr 15, 2026, 5:41:16 PMApr 15
to Marlon Facey, Bofeng Chen, chromiu...@luci-project-accounts.iam.gserviceaccount.com, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, penghuan...@chromium.org, devtools...@chromium.org, jz...@chromium.org, mar...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org, fgal...@chromium.org, feature-me...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, oshima...@chromium.org
Attention needed from Bofeng Chen and Marlon Facey
Sho Mizoe added 1 comment![]( "Open in Gerrit")
File ui/webui/resources/cr_components/composebox/composebox.ts
Line 355, Patchset 15 (Parent):
this.eventTracker_.removeAll();Sho Mizoe . unresolved
Do we want to keep this line (`this.eventTracker_.removeAll();`) in this class since we still have `this.eventTracker_.add(...)` call?
Related details
Attention is currently required from:
- Bofeng Chen
- Marlon Facey
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. |
Marlon Facey (Gerrit)
Apr 22, 2026, 10:55:31 AM (12 days ago) Apr 22
to Code Review Nudger, Sho Mizoe, Bofeng Chen, Chromium LUCI CQ, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, penghuan...@chromium.org, devtools...@chromium.org, jz...@chromium.org, mar...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org, fgal...@chromium.org, feature-me...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, oshima...@chromium.org
Attention needed from Bofeng Chen and Sho Mizoe
Marlon Facey added 1 comment![]( "Open in Gerrit")
File ui/webui/resources/cr_components/composebox/composebox.ts
Line 355, Patchset 15 (Parent):
this.eventTracker_.removeAll();Sho Mizoe . resolved
Do we want to keep this line (`this.eventTracker_.removeAll();`) in this class since we still have `this.eventTracker_.add(...)` call?
Attention is currently required from:
- Bofeng Chen
- Sho Mizoe
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Marlon Facey (Gerrit)
Apr 27, 2026, 11:26:06 AM (7 days ago) Apr 27
to Paul Adedeji, Code Review Nudger, Sho Mizoe, Chromium LUCI CQ, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, penghuan...@chromium.org, devtools...@chromium.org, jz...@chromium.org, mar...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org, fgal...@chromium.org, feature-me...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, oshima...@chromium.org
Attention needed from Paul Adedeji and Sho Mizoe
Marlon Facey voted Commit-Queue+1![]( "Open in Gerrit")
Attention is currently required from:
- Paul Adedeji
- Sho Mizoe
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Paul Adedeji (Gerrit)
Apr 27, 2026, 11:45:50 AM (7 days ago) Apr 27
to Marlon Facey, Bofeng Chen, Code Review Nudger, Sho Mizoe, Chromium LUCI CQ, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, penghuan...@chromium.org, devtools...@chromium.org, jz...@chromium.org, mar...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org, fgal...@chromium.org, feature-me...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, oshima...@chromium.org
Attention needed from Bofeng Chen, Marlon Facey and Sho Mizoe
Paul Adedeji added 4 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 17 (Latest):
Paul Adedeji . unresolved
Logic mostly lgtm, but what's the plan for adding test coverage? `omnibox_composebox.html.ts` and `omnibox_composebox.ts` have a lot of uncovered lines.
File chrome/browser/resources/omnibox_popup/omnibox_composebox.html.ts
Line 44, Patchset 17 (Latest):
.toolMode="${this.inputState?.activeTool || ToolMode.kUnspecified}"Paul Adedeji . unresolved
optional: Might be cleaner to make a property for this in `omnibox_composebox.ts`, since we're using properties for the rest of these.
File chrome/browser/resources/omnibox_popup/omnibox_composebox.ts
Line 109, Patchset 17 (Latest):
// TODO(crbug.com/486707998): Implement when carousel is added.
private addFileFromAttachment_(fileAttachment: FileAttachment) {
return fileAttachment;
}
// TODO(crbug.com/486707998): Implement when carousel is added.
private addTabFromAttachment_(tabAttachment: TabAttachment) {
return tabAttachment;
}Paul Adedeji . unresolved
What are these methods eventually supposed to do?
Line 120, Patchset 17 (Latest):
override shouldShowDivider(): boolean {Paul Adedeji . unresolved
optional: should this be placed with the other overrides near the top of this file (that start on line 66)?
Related details
Attention is currently required from:
- Bofeng Chen
- Marlon Facey
- Sho Mizoe
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. |
Marlon Facey (Gerrit)
Apr 27, 2026, 4:55:17 PM (7 days ago) Apr 27
to Paul Adedeji, Code Review Nudger, Sho Mizoe, Chromium LUCI CQ, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, penghuan...@chromium.org, devtools...@chromium.org, jz...@chromium.org, mar...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org, fgal...@chromium.org, feature-me...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, oshima...@chromium.org
Attention needed from Paul Adedeji and Sho Mizoe
Marlon Facey voted and added 4 comments![]( "Open in Gerrit")
Votes added by Marlon Facey
| Commit-Queue | +1 |
Logic mostly lgtm, but what's the plan for adding test coverage? `omnibox_composebox.html.ts` and `omnibox_composebox.ts` have a lot of uncovered lines.
Marlon Facey
Added a few tests. There isn't too much we can test right now since only the input and dropdown exist right now.
File chrome/browser/resources/omnibox_popup/omnibox_composebox.html.ts
Line 44, Patchset 17:
.toolMode="${this.inputState?.activeTool || ToolMode.kUnspecified}"Paul Adedeji . resolved
optional: Might be cleaner to make a property for this in `omnibox_composebox.ts`, since we're using properties for the rest of these.
Marlon Facey
That's a fair point. I prefer to keep it this way so we don't have to update the property in the lifecycle methods. We could make a computeToolMode but that also seems unnecessary.
File chrome/browser/resources/omnibox_popup/omnibox_composebox.ts
// TODO(crbug.com/486707998): Implement when carousel is added.
private addFileFromAttachment_(fileAttachment: FileAttachment) {
return fileAttachment;
}
// TODO(crbug.com/486707998): Implement when carousel is added.
private addTabFromAttachment_(tabAttachment: TabAttachment) {
return tabAttachment;
}Paul Adedeji . unresolved
What are these methods eventually supposed to do?
Marlon Facey
These are for uploading files and tabs. I left them unimplemented so that the addSearchContext method could be kept the unchanged.
Line 120, Patchset 17:
override shouldShowDivider(): boolean {Paul Adedeji . unresolved
optional: should this be placed with the other overrides near the top of this file (that start on line 66)?
Marlon Facey
Hmm I was planning to keep the Embedder-provided methods for DOM and Mojo access in one place and the methods for determining if an element should show in a separate place (below the functional methods). Lmk if you think it's better otherwise.
Related details
Attention is currently required from:
- Paul Adedeji
- Sho Mizoe
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. |
Paul Adedeji (Gerrit)
Apr 28, 2026, 11:24:47 AM (6 days ago) Apr 28
to Marlon Facey, Code Review Nudger, Sho Mizoe, Chromium LUCI CQ, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, penghuan...@chromium.org, devtools...@chromium.org, jz...@chromium.org, mar...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org, fgal...@chromium.org, feature-me...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, oshima...@chromium.org
Attention needed from Marlon Facey and Sho Mizoe
Paul Adedeji added 7 comments![]( "Open in Gerrit")
File chrome/browser/resources/omnibox_popup/omnibox_composebox.ts
Line 109, Patchset 17:
// TODO(crbug.com/486707998): Implement when carousel is added.
private addFileFromAttachment_(fileAttachment: FileAttachment) {
return fileAttachment;
}
// TODO(crbug.com/486707998): Implement when carousel is added.
private addTabFromAttachment_(tabAttachment: TabAttachment) {
return tabAttachment;
}Paul Adedeji . resolved
Marlon FaceyWhat are these methods eventually supposed to do?
These are for uploading files and tabs. I left them unimplemented so that the addSearchContext method could be kept the unchanged.
Paul Adedeji
Ah ok got it.
Line 120, Patchset 17:
override shouldShowDivider(): boolean {Paul Adedeji . resolved
Marlon Faceyoptional: should this be placed with the other overrides near the top of this file (that start on line 66)?
Hmm I was planning to keep the Embedder-provided methods for DOM and Mojo access in one place and the methods for determining if an element should show in a separate place (below the functional methods). Lmk if you think it's better otherwise.
Paul Adedeji
Got it, that makes sense to me.
File chrome/test/data/webui/omnibox_popup/omnibox_composebox_test.ts
Line 41, Patchset 20 (Latest):
await omniboxComposebox.updateComplete;Paul Adedeji . unresolved
In the past, the WebUI team has told me to prefer `microtasksFinished()` in tests... although the [official documentation](https://chromium.googlesource.com/chromium/src/+/HEAD/docs/webui/webui_using_lit.md#testing) does support `updateComplete` if the tests in this file can't use "microtasksFinished()". I wonder if all the `updateComplete` usages in this file could be replaced by "microtasksFinished()".
Line 44, Patchset 20 (Latest):
test(
'Shift+Enter allows inserting a newline when input is focused and not empty',
async () => {
const composeboxDiv =
omniboxComposebox.shadowRoot!.querySelector('#composebox');
assertTrue(!!composeboxDiv);
omniboxComposebox.input = 'Some text';
await omniboxComposebox.updateComplete;
const inputElement = omniboxComposebox.getInputElement();
inputElement.inputElement.focus();
let preventDefaultCalled = false;
const event = new KeyboardEvent('keydown', {
key: 'Enter',
shiftKey: true,
bubbles: true,
cancelable: true,
});
Object.defineProperty(event, 'preventDefault', {
value: () => {
preventDefaultCalled = true;
},
});
assertEquals(omniboxComposebox.getActiveElement(), inputElement);
composeboxDiv.dispatchEvent(event);
assertFalse(
preventDefaultCalled, 'preventDefault should not be called');
});Paul Adedeji . unresolved
nit: `#composebox` is exported in `omnibox_composebox.ts`, so you can use `omniboxComposebox.$.composebox` instead, at which point you would no longer need to assert its existence. And you would no longer need the message parts of the assertions, since there would only be one boolean assertion of the test
```suggestion
test(
'Shift+Enter allows inserting a newline when input is focused and not empty',
async () => {
omniboxComposebox.input = 'Some text';
await omniboxComposebox.updateComplete;
const inputElement = omniboxComposebox.getInputElement();
inputElement.inputElement.focus();
let preventDefaultCalled = false;
const event = new KeyboardEvent('keydown', {
key: 'Enter',
shiftKey: true,
bubbles: true,
cancelable: true,
});
Object.defineProperty(event, 'preventDefault', {
value: () => {
preventDefaultCalled = true;
},
});assertEquals(omniboxComposebox.getActiveElement(), inputElement);
const composeboxDiv = omniboxComposebox.$.composebox;
composeboxDiv.dispatchEvent(event);
assertFalse(preventDefaultCalled);
});
```
Line 80, Patchset 20 (Latest):
'Enter prevents inserting a newline and attempts to submit query when focus is not in dropdown',Paul Adedeji . unresolved
nit: two characters over the limit, consider shortening or breaking up into two lines
```suggestion
'Enter prevents inserting a newline and attempts to submit query when focus not in dropdown',
```
Line 158, Patchset 20 (Latest):
await omniboxComposebox.updateComplete;Paul Adedeji . unresolved
Out of curiousity, does this test fail without this line? Same for line 170.
File ui/webui/resources/cr_components/composebox/composebox.html.ts
Line 125, Patchset 20 (Latest):
${this.shouldShowSubmitButton() && this.searchboxLayoutMode === 'TallBottomContext' ? html`Paul Adedeji . unresolved
optional (out of CL's scope): I think the "html" part of this line goes over the character limit.
Related details
Attention is currently required from:
- Marlon Facey
- Sho Mizoe
Marlon Facey (Gerrit)
Apr 29, 2026, 4:38:46 PM (5 days ago) Apr 29
to Paul Adedeji, Code Review Nudger, Sho Mizoe, Chromium LUCI CQ, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, penghuan...@chromium.org, devtools...@chromium.org, jz...@chromium.org, mar...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org, fgal...@chromium.org, feature-me...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, oshima...@chromium.org
Attention needed from Paul Adedeji and Sho Mizoe
Marlon Facey voted and added 5 comments![]( "Open in Gerrit")
Votes added by Marlon Facey
| Commit-Queue | +1 |
5 comments
File chrome/test/data/webui/omnibox_popup/omnibox_composebox_test.ts
In the past, the WebUI team has told me to prefer `microtasksFinished()` in tests... although the [official documentation](https://chromium.googlesource.com/chromium/src/+/HEAD/docs/webui/webui_using_lit.md#testing) does support `updateComplete` if the tests in this file can't use "microtasksFinished()". I wonder if all the `updateComplete` usages in this file could be replaced by "microtasksFinished()".
Marlon Facey
Done
Line 44, Patchset 20:
test(Paul Adedeji . resolved
Marlon Facey
Done
Line 80, Patchset 20:
'Enter prevents inserting a newline and attempts to submit query when focus is not in dropdown',Paul Adedeji . resolved
nit: two characters over the limit, consider shortening or breaking up into two lines
```suggestion
'Enter prevents inserting a newline and attempts to submit query when focus not in dropdown',
```
Marlon Facey
Done
Out of curiousity, does this test fail without this line? Same for line 170.
Marlon Facey
Done
File ui/webui/resources/cr_components/composebox/composebox.html.ts
Line 125, Patchset 20:
${this.shouldShowSubmitButton() && this.searchboxLayoutMode === 'TallBottomContext' ? html`Paul Adedeji . resolved
optional (out of CL's scope): I think the "html" part of this line goes over the character limit.
Attention is currently required from:
- Paul Adedeji
- Sho Mizoe
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Paul Adedeji (Gerrit)
Apr 30, 2026, 1:22:16 PM (4 days ago) Apr 30
to Marlon Facey, Code Review Nudger, Sho Mizoe, Chromium LUCI CQ, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, penghuan...@chromium.org, devtools...@chromium.org, jz...@chromium.org, mar...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org, fgal...@chromium.org, feature-me...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, oshima...@chromium.org
Attention needed from Marlon Facey and Sho Mizoe
Paul Adedeji added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 23 (Latest):
Paul Adedeji . resolved
Will wait for dry run to pass before stamping
Related details
Attention is currently required from:
- Marlon Facey
- Sho Mizoe
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Marlon Facey (Gerrit)
Apr 30, 2026, 3:58:34 PM (4 days ago) Apr 30
to Paul Adedeji, Code Review Nudger, Sho Mizoe, Chromium LUCI CQ, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, penghuan...@chromium.org, devtools...@chromium.org, jz...@chromium.org, mar...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org, fgal...@chromium.org, feature-me...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, oshima...@chromium.org
Attention needed from Sho Mizoe
Marlon Facey voted Commit-Queue+1![]( "Open in Gerrit")
Attention is currently required from:
- Sho Mizoe
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ryan Akiyama (Gerrit)
May 1, 2026, 2:20:30 PM (3 days ago) May 1
to Marlon Facey, Paul Adedeji, Code Review Nudger, Sho Mizoe, Chromium LUCI CQ, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, penghuan...@chromium.org, devtools...@chromium.org, jz...@chromium.org, mar...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org, fgal...@chromium.org, feature-me...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, oshima...@chromium.org
Attention needed from Marlon Facey and Sho Mizoe
Ryan Akiyama voted and added 3 comments![]( "Open in Gerrit")
Votes added by Ryan Akiyama
| Code-Review | +1 |
3 comments
Commit Message
Line 15, Patchset 25 (Latest):
https://screencast/cast/NTM3MjYwNTUxODMxNTUyMHw5NTgyYjM3Mi03YwRyan Akiyama . unresolved
nit: http
File chrome/test/data/webui/omnibox_popup/omnibox_composebox_test.ts
Line 57, Patchset 25 (Latest):
let preventDefaultCalled = false;
const event = new KeyboardEvent('keydown', {
key: 'Enter',
shiftKey: true,
bubbles: true,
cancelable: true,
});Ryan Akiyama . unresolved
nit: I'm having trouble following the AAA structure, understand that it might be unavoidable for the sections to interchange a few times, but maybe comments will help. Similar for the two tests below.
File ui/webui/resources/cr_components/composebox/composebox_mixin.ts
Line 476, Patchset 25 (Latest):
onAutocompleteResultChanged(result: AutocompleteResult) {Ryan Akiyama . unresolved
Checking if we still need this method in `composebox.ts`, or will it be removed in a future CL? Similar for any other method that was moved over to the mixin and not removed from `composebox.ts`.
Related details
Attention is currently required from:
- Marlon Facey
- Sho Mizoe
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. |
Marlon Facey (Gerrit)
May 1, 2026, 3:18:31 PM (3 days ago) May 1
to Ryan Akiyama, Paul Adedeji, Code Review Nudger, Sho Mizoe, Chromium LUCI CQ, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, penghuan...@chromium.org, devtools...@chromium.org, jz...@chromium.org, mar...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org, fgal...@chromium.org, feature-me...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, oshima...@chromium.org
Attention needed from Sho Mizoe
Marlon Facey voted and added 3 comments![]( "Open in Gerrit")
Votes added by Marlon Facey
| Commit-Queue | +1 |
3 comments
Commit Message
Line 15, Patchset 25:
https://screencast/cast/NTM3MjYwNTUxODMxNTUyMHw5NTgyYjM3Mi03YwRyan Akiyama . resolved
Marlon Faceynit: http
Done
File chrome/test/data/webui/omnibox_popup/omnibox_composebox_test.ts
Line 57, Patchset 25:
let preventDefaultCalled = false;
const event = new KeyboardEvent('keydown', {
key: 'Enter',
shiftKey: true,
bubbles: true,
cancelable: true,
});Ryan Akiyama . resolved
nit: I'm having trouble following the AAA structure, understand that it might be unavoidable for the sections to interchange a few times, but maybe comments will help. Similar for the two tests below.
Marlon Facey
Is it more clear with the comments now?
File ui/webui/resources/cr_components/composebox/composebox_mixin.ts
Line 476, Patchset 25:
onAutocompleteResultChanged(result: AutocompleteResult) {Ryan Akiyama . resolved
Checking if we still need this method in `composebox.ts`, or will it be removed in a future CL? Similar for any other method that was moved over to the mixin and not removed from `composebox.ts`.
Marlon Facey
There are some embedder specific portions of this in composebox.ts. So I left the implementation in composebox.ts to prevent regressions.
Related details
Attention is currently required from:
- Sho Mizoe
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ryan Akiyama (Gerrit)
May 1, 2026, 4:11:59 PM (3 days ago) May 1
to Marlon Facey, Paul Adedeji, Code Review Nudger, Sho Mizoe, Chromium LUCI CQ, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, penghuan...@chromium.org, devtools...@chromium.org, jz...@chromium.org, mar...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org, fgal...@chromium.org, feature-me...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, oshima...@chromium.org
Attention needed from Marlon Facey and Sho Mizoe
Ryan Akiyama voted and added 1 comment![]( "Open in Gerrit")
Votes added by Ryan Akiyama
| Code-Review | +1 |
1 comment
File chrome/test/data/webui/omnibox_popup/omnibox_composebox_test.ts
Line 57, Patchset 25:
let preventDefaultCalled = false;
const event = new KeyboardEvent('keydown', {
key: 'Enter',
shiftKey: true,
bubbles: true,
cancelable: true,
});Ryan Akiyama . resolved
Marlon Faceynit: I'm having trouble following the AAA structure, understand that it might be unavoidable for the sections to interchange a few times, but maybe comments will help. Similar for the two tests below.
Is it more clear with the comments now?
Attention is currently required from:
- Marlon Facey
- Sho Mizoe
Marlon Facey (Gerrit)
10:45 AM (5 hours ago) 10:45 AM
to Ryan Akiyama, Paul Adedeji, Code Review Nudger, Sho Mizoe, Chromium LUCI CQ, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, penghuan...@chromium.org, devtools...@chromium.org, jz...@chromium.org, mar...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org, fgal...@chromium.org, feature-me...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, oshima...@chromium.org
Attention needed from Sho Mizoe
Marlon Facey voted Commit-Queue+2![]( "Open in Gerrit")
Attention is currently required from:
- Sho Mizoe
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Marlon Facey (Gerrit)
10:56 AM (5 hours ago) 10:56 AM
to Ryan Akiyama, Paul Adedeji, Code Review Nudger, Sho Mizoe, Chromium LUCI CQ, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, penghuan...@chromium.org, devtools...@chromium.org, jz...@chromium.org, mar...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org, fgal...@chromium.org, feature-me...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, oshima...@chromium.org
Attention needed from Sho Mizoe
Marlon Facey voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention is currently required from:
- Sho Mizoe
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
12:19 PM (3 hours ago) 12:19 PM
to Marlon Facey, Ryan Akiyama, Paul Adedeji, Code Review Nudger, Sho Mizoe, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, net-r...@chromium.org, cblume...@chromium.org, penghuan...@chromium.org, devtools...@chromium.org, jz...@chromium.org, mar...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org, fgal...@chromium.org, feature-me...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org, oshima...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Unreviewed changes
27 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Change information
Commit message:
[Omnibox Next] Add context entrypoint and dropdown
Add the context entrypoint and dropdown to the omnibox composebox fork.
Moves logic to the mixin that is needed to determine visibility of
certain elements. This does not contain the actual context entrypoint
just the parent container as of now. It also moves some lifecycle
methods to the mixin.
http://screencast/cast/NTM3MjYwNTUxODMxNTUyMHw5NTgyYjM3Mi03Yw
Bug: 486707998
Change-Id: I7086f28c497c6c782bc1affd426d952b27adae14
Commit-Queue: Marlon Facey <mfa...@chromium.org>
Reviewed-by: Ryan Akiyama <ryana...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1624738}
Files:
- M chrome/browser/resources/omnibox_popup/aim_app.css
- M chrome/browser/resources/omnibox_popup/aim_app.html.ts
- M chrome/browser/resources/omnibox_popup/omnibox_composebox.html.ts
- M chrome/browser/resources/omnibox_popup/omnibox_composebox.ts
- M chrome/browser/resources/omnibox_popup/omnibox_popup.ts
- M chrome/test/data/webui/omnibox_popup/BUILD.gn
- A chrome/test/data/webui/omnibox_popup/omnibox_composebox_test.ts
- M chrome/test/data/webui/omnibox_popup/omnibox_popup_browsertest.cc
- M ui/webui/resources/cr_components/composebox/composebox.html.ts
- M ui/webui/resources/cr_components/composebox/composebox.ts
- M ui/webui/resources/cr_components/composebox/composebox_mixin.ts
Change size: L
Delta: 11 files changed, 498 insertions(+), 118 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Ryan Akiyama
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages