GitLab

Previously, reverting the omnibox's selected match (e.g. by pressing escape)
when the default match is in keyword mode resulted in incorrect behavior
regarding the user text, autocomplete text, and text styling:

(1) The user text has its first word removed for each time keyword mode was
entered. If the results popup included a navigation result, pressing
up/down to select the navigation result leaves keyword mode. Pressing
up/down to select a search result or pressing escape to revert the omnibox text
reenters keyword mode. Each time keyword mode is entered, the user text drops
the first word. E.g. if the 2nd result is a navigation result, and the third
result is a search result, pressing <down><down><up><escape> would enter keyword
mode twice, and drop the first two words of the user text.

(2) The user text would be styled as a URL (grey font color) if the result
selected before reverting was a URL result.

(3) The autocomplete text is not restored.

This CL addresses (1) and (3). (2) is partially addressed as well;
for cases where the default match has autocomplete text, restoring it correctly
also restores the user text styling. The fix is 2 part:

(1) Before, the first word of the user text was stripped upon entering keyword
mode because we assumed the first word must be the keyword word. However, this
assumption is only true when accepting a keyword (e.g. pressing <space> or
<tab>), and not when entering keyword mode by reverting the omnibox's selected
match (e.g. pressing <escape>). This CL moves the stripping call from
OmniboxEditModel::OnPopupDataChanged, which is called in both cases, to
OmniboxEditModel::OnCurrentMatchChanged, which is only called in the first case.

(2) Before, the cursor was reset to the beginning of the user text when entering
keyword mode while the default match was either already selected or becoming
selected. The first occurs when (a) inserting a space between a keyword name and
a search string, while the latter occurs when (b) reverting the omnibox's
selected match. This was desired behavior in case (a), but undesired in case
(b). We incorrectly assumed it was ok to do this in case (b) because we would be
reverting the text anyways. However, the revert happens prior to this, resulting
in incorrect autocomplete selection. This CL conditions the cursor reset on not
having autocomplete text.

Bug: 702867
Change-Id: Iac0bdae7e17c86b3e0feb3075dd916d85e5d7e39
Reviewed-on: https://chromium-review.googlesource.com/c/1458697
Commit-Queue: manuk hovanesian <manukh@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#630105}