Commit b5759239 authored by manukh's avatar manukh Committed by Chromium LUCI CQ

[omnibox] [rich-autocompletion] Clear additional text on popup close.

Closing the omnibox popup can change the selected suggestion. This
occurs in 3 cases:

1) Search what you type suggestions that look like URLs. E.g. after
selecting the search-what-you-typed suggestion 'x.com' and closing the
popup, the omnibox will navigate to 'x.com' instead of searching the
default search engine for 'x.com'

2) Title autocompletions for inputs that look like searches. E.g., for
the suggestion 'jupiter - wi[kipedia]', after closing the popup, the
omnibox will display 'jupiter - wikipedia (wikipedia.org/wiki/Jupiter)'
but will search for 'jupiter - wikipedia' instead of navigating to
'wikipedia.org/wiki/Jupiter'.

3) Title autocompletions for inputs that look like a different URL than
their URLs. E.g., for a bookmark titled 'x.com' with URL 'y.com', after
closing the popup, the omnibox will display 'x.com (y.com)' but actually
navigate to 'x.com' instead of 'y.com'.

The first case isn't problematic, because closing the popup updates the
omnibox favicon, and there's no additional text. Cases 2 and 3 are
problematic because the additional text from before closing the omnibox
no longer represents where the omnibox will actually navigate to after
closing the omnibox. This CL addresses this by clearing the additional
text when closing the popup.

Perhaps the preferred long-term solution would be to prevent the
selected suggestion from changing when closing the popup. But for the
time being, this CL at least updates the view to correspond to the
updated suggestion.

Bug: 1062446
Change-Id: If5c4ef25abbd73d54540eeb8cb5b85b4db307107
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2628011Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Commit-Queue: manuk hovanesian <manukh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845346}
parent c3f529c4
...@@ -71,6 +71,13 @@ void OmniboxController::OnResultChanged(AutocompleteController* controller, ...@@ -71,6 +71,13 @@ void OmniboxController::OnResultChanged(AutocompleteController* controller,
// Accept the temporary text as the user text, because it makes little sense // Accept the temporary text as the user text, because it makes little sense
// to have temporary text when the popup is closed. // to have temporary text when the popup is closed.
omnibox_edit_model_->AcceptTemporaryTextAsUserText(); omnibox_edit_model_->AcceptTemporaryTextAsUserText();
// Closing the popup can change the default suggestion. This usually occurs
// when it's unclear whether the input represents a search or URL; e.g.,
// 'a.com/b c' or when title autocompleting. Clear the additional text to
// avoid suggesting the omnibox contains a URL suggestion when that may no
// longer be the case; i.e. when the default suggestion changed from a URL
// to a search suggestion upon closing the popup.
omnibox_edit_model_->ClearAdditionalText();
} }
// Note: The client outlives |this|, so bind a weak pointer to the callback // Note: The client outlives |this|, so bind a weak pointer to the callback
......
...@@ -1107,6 +1107,10 @@ void OmniboxEditModel::ClearKeyword() { ...@@ -1107,6 +1107,10 @@ void OmniboxEditModel::ClearKeyword() {
} }
} }
void OmniboxEditModel::ClearAdditionalText() {
view_->SetAdditionalText(base::string16());
}
void OmniboxEditModel::OnSetFocus(bool control_down) { void OmniboxEditModel::OnSetFocus(bool control_down) {
last_omnibox_focus_ = base::TimeTicks::Now(); last_omnibox_focus_ = base::TimeTicks::Now();
user_input_since_focus_ = false; user_input_since_focus_ = false;
......
...@@ -288,6 +288,9 @@ class OmniboxEditModel { ...@@ -288,6 +288,9 @@ class OmniboxEditModel {
// Clears the current keyword. // Clears the current keyword.
void ClearKeyword(); void ClearKeyword();
// Clears additional text.
void ClearAdditionalText();
// Returns the current autocomplete result. This logic should in the future // Returns the current autocomplete result. This logic should in the future
// live in AutocompleteController but resides here for now. This method is // live in AutocompleteController but resides here for now. This method is
// used by AutomationProvider::AutocompleteEditGetMatches. // used by AutomationProvider::AutocompleteEditGetMatches.
......
...@@ -105,9 +105,7 @@ class OmniboxView { ...@@ -105,9 +105,7 @@ class OmniboxView {
bool update_popup); bool update_popup);
// Sets the window text and the caret position. |notify_text_changed| is true // Sets the window text and the caret position. |notify_text_changed| is true
// if the model should be notified of the change. If rich autocompletion is // if the model should be notified of the change. Clears the additional text.
// enabled, |additional_text| is displayed in a non-editable views::Label
// adjacent to the omnibox.
virtual void SetWindowTextAndCaretPos(const base::string16& text, virtual void SetWindowTextAndCaretPos(const base::string16& text,
size_t caret_pos, size_t caret_pos,
bool update_popup, bool update_popup,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment