Commit 6b275ec9 authored by Tommy Li's avatar Tommy Li Committed by Commit Bot

[omnibox] On-Focus Suggestions / ZeroSuggest - Fix revert to user text

Currently, the process of reverting to the user text is broken for
on-focus suggestions.

After using the arrow keys to choose a match, pressing Escape does not
work. Escape selects the top match, which normally works, but in
ZeroSuggest, the textfield contents could be a non-prefix of the first
suggestion.

This CL reverts the textfield to the input that was sent to the
Autocomplete controller, which should work for on-focus suggestions.

This also adds a test.

Bug: 1012990, 996516, 963173
Change-Id: Id19aaf961591dc0c81bf9083b639233c5f8e4420
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1865551Reviewed-by: default avatarKevin Bailey <krb@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707068}
parent 22880077
......@@ -1287,7 +1287,7 @@ void OmniboxEditModel::OnPopupDataChanged(const base::string16& text,
view_->OnInlineAutocompleteTextCleared();
const base::string16& user_text =
user_input_in_progress_ ? user_text_ : view_->GetText();
user_input_in_progress_ ? user_text_ : input_.text();
if (keyword_state_changed && is_keyword_selected() &&
inline_autocomplete_text_.empty()) {
// If we reach here, the user most likely entered keyword mode by inserting
......@@ -1539,6 +1539,15 @@ void OmniboxEditModel::RevertTemporaryTextAndPopup() {
if (popup_model())
popup_model()->ResetToDefaultMatch();
// If user input is not in progress, we are reverting an on-focus suggestion.
// Set the window text back to the original input, rather than the top match.
// The original selection will be restored in OnRevertTemporaryText() below.
if (!user_input_in_progress_) {
view_->SetWindowTextAndCaretPos(input_.text(), /*caret_pos=*/0,
/*update_popup=*/false,
/*notify_text_changed=*/true);
}
const AutocompleteMatch& match = CurrentMatch(nullptr);
view_->OnRevertTemporaryText(match.fill_into_edit, match);
}
......
......@@ -279,6 +279,7 @@ TEST_F(OmniboxEditModelTest, RespectUnelisionInZeroSuggest) {
// Test that we don't clobber the unelided text with inline autocomplete text.
EXPECT_EQ(base::string16(), view()->inline_autocomplete_text());
model()->ShowOnFocusSuggestionsIfAutocompleteIdle();
model()->OnPopupDataChanged(base::string16(), /*is_temporary_text=*/false,
base::string16(), false);
EXPECT_EQ(base::ASCIIToUTF16("https://www.example.com/"), view()->GetText());
......@@ -287,6 +288,28 @@ TEST_F(OmniboxEditModelTest, RespectUnelisionInZeroSuggest) {
}
#endif // !defined(OS_IOS)
TEST_F(OmniboxEditModelTest, RevertZeroSuggestTemporaryText) {
location_bar_model()->set_url(GURL("https://www.example.com/"));
location_bar_model()->set_url_for_display(
base::ASCIIToUTF16("https://www.example.com/"));
EXPECT_TRUE(model()->ResetDisplayTexts());
model()->Revert();
// Simulate getting ZeroSuggestions and arrowing to a different match.
view()->SelectAll(true);
model()->ShowOnFocusSuggestionsIfAutocompleteIdle();
model()->OnPopupDataChanged(base::ASCIIToUTF16("fake_temporary_text"),
/*is_temporary_text=*/true, base::string16(),
false);
// Test that reverting brings back the original input text.
EXPECT_TRUE(model()->OnEscapeKeyPressed());
EXPECT_EQ(base::ASCIIToUTF16("https://www.example.com/"), view()->GetText());
EXPECT_FALSE(model()->user_input_in_progress());
EXPECT_TRUE(view()->IsSelectAll());
}
// This verifies the fix for a bug where calling OpenMatch() with a valid
// alternate nav URL would fail a DCHECK if the input began with "http://".
// The failure was due to erroneously trying to strip the scheme from the
......
......@@ -46,6 +46,9 @@ void TestOmniboxView::OnTemporaryTextMaybeChanged(
bool save_original_selection,
bool notify_text_changed) {
text_ = display_text;
if (save_original_selection)
saved_temporary_selection_ = selection_;
}
bool TestOmniboxView::OnInlineAutocompleteTextMaybeChanged(
......@@ -67,6 +70,11 @@ void TestOmniboxView::OnInlineAutocompleteTextCleared() {
inline_autocomplete_text_.clear();
}
void TestOmniboxView::OnRevertTemporaryText(const base::string16& display_text,
const AutocompleteMatch& match) {
selection_ = saved_temporary_selection_;
}
bool TestOmniboxView::OnAfterPossibleChange(bool allow_keyword_ui_change) {
return false;
}
......
......@@ -58,7 +58,7 @@ class TestOmniboxView : public OmniboxView {
size_t user_text_length) override;
void OnInlineAutocompleteTextCleared() override;
void OnRevertTemporaryText(const base::string16& display_text,
const AutocompleteMatch& match) override {}
const AutocompleteMatch& match) override;
void OnBeforePossibleChange() override {}
bool OnAfterPossibleChange(bool allow_keyword_ui_change) override;
gfx::NativeView GetNativeView() const override;
......@@ -73,6 +73,7 @@ class TestOmniboxView : public OmniboxView {
base::string16 text_;
base::string16 inline_autocomplete_text_;
gfx::Range selection_;
gfx::Range saved_temporary_selection_;
DISALLOW_COPY_AND_ASSIGN(TestOmniboxView);
};
......
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