Align Android Omnibox word deletion with desktop [chromium/src : main]
0 views
Skip to first unread message
Sky Malice (Gerrit)
Jul 7, 2026, 2:45:27 PM (23 hours ago) Jul 7
to Charles Cai, Patrick Noland, Chromium LUCI CQ, chromium...@chromium.org, christia...@chromium.org, ender...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org
Attention needed from Charles Cai and Patrick Noland
Sky Malice voted and added 2 comments![]( "Open in Gerrit")
Votes added by Sky Malice
| Code-Review | +1 |
2 comments
Patchset-level comments
File-level comment, Patchset 2 (Latest):
Sky Malice . resolved
Wow, very trick problem, and a nice looking CL. Looks really great Charles!
File components/omnibox/browser/omnibox_word_boundary.cc
File-level comment, Patchset 2 (Latest):
Sky Malice . unresolved
I the real WLM code that does this too difficult to re-use? Should this file get a unit test?
Related details
Attention is currently required from:
- Charles Cai
- Patrick Noland
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. |
Charles Cai (Gerrit)
Jul 7, 2026, 4:02:31 PM (21 hours ago) Jul 7
to Sky Malice, Patrick Noland, Chromium LUCI CQ, chromium...@chromium.org, christia...@chromium.org, ender...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org
Attention needed from Patrick Noland and Sky Malice
Charles Cai voted and added 1 comment![]( "Open in Gerrit")
Votes added by Charles Cai
| Commit-Queue | +1 |
1 comment
File components/omnibox/browser/omnibox_word_boundary.cc
I the real WLM code that does this too difficult to re-use? Should this file get a unit test?
Charles Cai
1. Yes. WLM uses RenderText, and a whole lot of dependency issues would appear if we re-use that.
2. Right, I think this logic should have a unit test. Done.
Related details
Attention is currently required from:
- Patrick Noland
- Sky Malice
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sky Malice (Gerrit)
Jul 7, 2026, 4:17:58 PM (21 hours ago) Jul 7
to Charles Cai, Patrick Noland, Chromium LUCI CQ, chromium...@chromium.org, christia...@chromium.org, ender...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org
Attention needed from Charles Cai and Patrick Noland
Sky Malice voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Charles Cai
- Patrick Noland
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Patrick Noland (Gerrit)
Jul 7, 2026, 5:37:22 PM (20 hours ago) Jul 7
to Charles Cai, Sky Malice, Chromium LUCI CQ, chromium...@chromium.org, christia...@chromium.org, ender...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org
Attention needed from Charles Cai
Patrick Noland voted and added 2 comments![]( "Open in Gerrit")
Votes added by Patrick Noland
| Code-Review | +1 |
2 comments
File chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextModelBase.java
Line 250, Patchset 3 (Latest):
* @return Whether the given key event represents a delete-by-word gesture that should bePatrick Noland . unresolved
```suggestion
* Returns Whether the given key event represents a delete-by-word gesture that should be
```
File components/omnibox/browser/omnibox_word_boundary_unittest.cc
Line 23, Patchset 3 (Latest):
TEST(OmniboxWordBoundaryTest, GetDeletionBoundary_Forward) {Patrick Noland . unresolved
Are there any trickier cases BreakIterator handles for us that we want to explicitly test?
Related details
Attention is currently required from:
- Charles Cai
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. |
Charles Cai (Gerrit)
9:42 AM (4 hours ago) 9:42 AM
to Patrick Noland, Sky Malice, Chromium LUCI CQ, chromium...@chromium.org, christia...@chromium.org, ender...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org
Charles Cai added 2 comments![]( "Open in Gerrit")
File chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextModelBase.java
Line 250, Patchset 3:
* @return Whether the given key event represents a delete-by-word gesture that should bePatrick Noland . resolved
```suggestion
* Returns Whether the given key event represents a delete-by-word gesture that should be
```
Charles Cai
Done
File components/omnibox/browser/omnibox_word_boundary_unittest.cc
Line 23, Patchset 3:
TEST(OmniboxWordBoundaryTest, GetDeletionBoundary_Forward) {Patrick Noland . resolved
Are there any trickier cases BreakIterator handles for us that we want to explicitly test?
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Charles Cai (Gerrit)
9:43 AM (4 hours ago) 9:43 AM
to Patrick Noland, Sky Malice, Chromium LUCI CQ, chromium...@chromium.org, christia...@chromium.org, ender...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org
Charles Cai voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
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. |
Chromium LUCI CQ (Gerrit)
10:44 AM (3 hours ago) 10:44 AM
to Charles Cai, Patrick Noland, Sky Malice, chromium...@chromium.org, christia...@chromium.org, ender...@chromium.org, jdonnel...@chromium.org, omnibox-...@chromium.org
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
3 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: components/omnibox/browser/omnibox_word_boundary_unittest.cc
Insertions: 13, Deletions: 0.
@@ -18,6 +18,13 @@
EXPECT_EQ(7, GetDeletionBoundary(u"google.com/path", 11, false));
EXPECT_EQ(0, GetDeletionBoundary(u"https://google.com", 8, false));
+
+ EXPECT_EQ(11, GetDeletionBoundary(u"google.com:8080", 15, false));
+
+ EXPECT_EQ(11, GetDeletionBoundary(u"google.com/path_/file.html", 17, false));
+
+ EXPECT_EQ(11, GetDeletionBoundary(u"[2001:db8::1]", 13, false));
+ EXPECT_EQ(6, GetDeletionBoundary(u"[2001:db8::1]", 11, false));
}
TEST(OmniboxWordBoundaryTest, GetDeletionBoundary_Forward) {
@@ -28,6 +35,12 @@
EXPECT_EQ(15, GetDeletionBoundary(u"google.com/path", 10, true));
EXPECT_EQ(14, GetDeletionBoundary(u"https://google.com", 5, true));
+
+ EXPECT_EQ(15, GetDeletionBoundary(u"google.com:8080", 10, true));
+
+ EXPECT_EQ(21, GetDeletionBoundary(u"google.com/path_/file.html", 16, true));
+
+ EXPECT_EQ(9, GetDeletionBoundary(u"[2001:db8::1]", 5, true));
}
} // namespace omnibox
```
```
The name of the file: chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextModelBase.java
Insertions: 2, Deletions: 2.
@@ -247,8 +247,8 @@
boolean shouldIgnoreAccessibilityEvent();
/**
- * @return Whether the given key event represents a delete-by-word gesture that should be
- * intercepted and handled by the model.
+ * Returns whether the given key event represents a delete-by-word gesture that should be
+ * intercepted and handled by the model.
*/
boolean isDeleteByWord(KeyEvent event);
}
```
Change information
Commit message:
Align Android Omnibox word deletion with desktop
This patch implements custom word deletion for Android Omnibox.
Previously these key events were consumed by the IME and handled using
Android's native rules. We override this so that word deletion is routed
to dispatchKeyEvent. A JNI bridge OmniboxWordBoundary is created to use
the base::i18n::BreakIterator for calculating the exact
desktop-consistent word boundaries. And we delete words according to
this.
This builds on top of forward deletion, so that it supports backward and
forward word deletion.
Bug: 514800370
Change-Id: Ie14df1fa466e79912f63e43ac65c8da8095f9b1f
Reviewed-by: Patrick Noland <pno...@chromium.org>
Commit-Queue: Charles Cai <char...@google.com>
Reviewed-by: Sky Malice <sk...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1658739}
Files:
- M chrome/browser/ui/android/omnibox/BUILD.gn
- M chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditText.java
- M chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextModelBase.java
- M chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/SpannableAutocompleteEditTextModel.java
- M chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/SpannableAutocompleteEditTextModelUnitTest.java
- M components/omnibox/browser/BUILD.gn
- A components/omnibox/browser/android/java/src/org/chromium/components/omnibox/OmniboxWordBoundary.java
- A components/omnibox/browser/omnibox_word_boundary.cc
- A components/omnibox/browser/omnibox_word_boundary.h
- A components/omnibox/browser/omnibox_word_boundary_unittest.cc
Change size: L
Delta: 10 files changed, 273 insertions(+), 0 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Patrick Noland, +1 by Sky Malice
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages