Commit c2c9d409 authored by Tommy Li's avatar Tommy Li Committed by Commit Bot

[omnibox] Fix losing HTTPS for elided URLs when user reloads page

Currently, for elided HTTPS URLs, (omnibox displays 'google.com'
instead of 'https://www.google.com'), when the user mouse clicks into
the omnibox and presses Enter, we make a request to http://google.com.

This is a regression introduced in this CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1854501

This CL is a partial revert of that CL. I think the reverted part wasn't
necessary to fix the original bug in the first place, and was likely
included by mistake.

This CL does the partial revert and also enhances our existing unit
test to make sure it won't happen again.

Bug: 1037889
Change-Id: Ia5e742c02ac35f7b013ce42bc01c1217304ab303
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1990253
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: default avatarKevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#730245}
parent ccfc1eb8
...@@ -1535,9 +1535,18 @@ void OmniboxEditModel::GetInfoForCurrentText(AutocompleteMatch* match, ...@@ -1535,9 +1535,18 @@ void OmniboxEditModel::GetInfoForCurrentText(AutocompleteMatch* match,
} }
if (!found_match_for_text) { if (!found_match_for_text) {
// For match generation, we use the unelided |url_for_editing_|, unless the
// user input is in progress or Query in Omnibox is active.
LocationBarModel* location_bar_model = controller()->GetLocationBarModel();
base::string16 text_for_match_generation = url_for_editing_;
if (user_input_in_progress() ||
location_bar_model->GetDisplaySearchTerms(nullptr)) {
text_for_match_generation = view_->GetText();
}
client_->GetAutocompleteClassifier()->Classify( client_->GetAutocompleteClassifier()->Classify(
MaybePrependKeyword(view_->GetText()), is_keyword_selected(), true, MaybePrependKeyword(text_for_match_generation), is_keyword_selected(),
GetPageClassification(), match, alternate_nav_url); true, GetPageClassification(), match, alternate_nav_url);
} }
} }
......
...@@ -338,26 +338,58 @@ TEST_F(OmniboxEditModelTest, AlternateNavHasHTTP) { ...@@ -338,26 +338,58 @@ TEST_F(OmniboxEditModelTest, AlternateNavHasHTTP) {
} }
TEST_F(OmniboxEditModelTest, CurrentMatch) { TEST_F(OmniboxEditModelTest, CurrentMatch) {
location_bar_model()->set_url(GURL("http://localhost/")); // Test the HTTP case.
location_bar_model()->set_url_for_display(base::ASCIIToUTF16("localhost")); {
model()->ResetDisplayTexts(); location_bar_model()->set_url(GURL("http://www.example.com/"));
model()->Revert(); location_bar_model()->set_url_for_display(
base::ASCIIToUTF16("example.com"));
model()->ResetDisplayTexts();
model()->Revert();
// iOS doesn't do elision in the textfield view.
#if defined(OS_IOS)
EXPECT_EQ(base::ASCIIToUTF16("http://www.example.com/"), view()->GetText());
#else
EXPECT_EQ(base::ASCIIToUTF16("example.com"), view()->GetText());
#endif
AutocompleteMatch match = model()->CurrentMatch(nullptr);
EXPECT_EQ(AutocompleteMatchType::URL_WHAT_YOU_TYPED, match.type);
EXPECT_TRUE(model()->CurrentTextIsURL());
EXPECT_EQ("http://www.example.com/", match.destination_url.spec());
}
// Tests that we use the formatted full URL instead of the elided URL to // Test that generating a match from an elided HTTPS URL doesn't drop the
// generate matches. // secure scheme.
{ {
location_bar_model()->set_url(GURL("https://www.google.com/"));
location_bar_model()->set_url_for_display(base::ASCIIToUTF16("google.com"));
model()->ResetDisplayTexts();
model()->Revert();
// iOS doesn't do elision in the textfield view.
#if defined(OS_IOS)
EXPECT_EQ(base::ASCIIToUTF16("https://www.google.com/"), view()->GetText());
#else
EXPECT_EQ(base::ASCIIToUTF16("google.com"), view()->GetText());
#endif
AutocompleteMatch match = model()->CurrentMatch(nullptr); AutocompleteMatch match = model()->CurrentMatch(nullptr);
EXPECT_EQ(AutocompleteMatchType::URL_WHAT_YOU_TYPED, match.type); EXPECT_EQ(AutocompleteMatchType::URL_WHAT_YOU_TYPED, match.type);
EXPECT_TRUE(model()->CurrentTextIsURL()); EXPECT_TRUE(model()->CurrentTextIsURL());
// Additionally verify we aren't accidentally dropping the HTTPS scheme.
EXPECT_EQ("https://www.google.com/", match.destination_url.spec());
} }
// Tests that when there is a Query in Omnibox, generate matches from the // Tests that when there is a Query in Omnibox, generate matches from the
// query, instead of the full formatted URL. // query, instead of the full formatted URL.
location_bar_model()->set_display_search_terms(base::ASCIIToUTF16("foobar"));
model()->ResetDisplayTexts();
model()->Revert();
{ {
location_bar_model()->set_display_search_terms(
base::ASCIIToUTF16("foobar"));
model()->ResetDisplayTexts();
model()->Revert();
AutocompleteMatch match = model()->CurrentMatch(nullptr); AutocompleteMatch match = model()->CurrentMatch(nullptr);
EXPECT_EQ(AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, match.type); EXPECT_EQ(AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, match.type);
EXPECT_FALSE(model()->CurrentTextIsURL()); EXPECT_FALSE(model()->CurrentTextIsURL());
......
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