Commit 81446466 authored by manuk hovanesian's avatar manuk hovanesian Committed by Commit Bot

Revert "omnibox: reset display url when re-navigating to the current page by clicking a link"

This reverts commit b7e58d9d.

Reason for revert: Introduced regression issues relating to omnibox text being cleared.

Original change's description:
> omnibox: reset display url when re-navigating to the current page by clicking a link
> 
> Previously, if navigating a link did not cause a url change (e.g. clicking on the `stackoverflow` logo on the `stackoverflow.com` homepage reloads the same page), then the display url remained unchanged. If the user had deleted or edited the url, his changes would remain. Now, the url is reset.
> 
> Bug: 830491
> Change-Id: If1aa07715175d7d5bd5b12a65d9e1d9529aca3b5
> Reviewed-on: https://chromium-review.googlesource.com/1103233
> Commit-Queue: manuk hovanesian <manukh@chromium.org>
> Reviewed-by: Tommy Li <tommycli@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#568831}

TBR=tommycli@chromium.org,manukh@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 830491
Change-Id: If20093795454513c9734fa2c8ed686843e83a683
Reviewed-on: https://chromium-review.googlesource.com/1113879Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Commit-Queue: manuk hovanesian <manukh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570165}
parent 42baf207
......@@ -445,7 +445,14 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, DISABLED_BrowserAccelerators) {
#endif
}
IN_PROC_BROWSER_TEST_F(OmniboxViewTest, PopupAccelerators) {
// Fails on Linux. http://crbug.com/408634
#if defined(OS_LINUX)
#define MAYBE_PopupAccelerators DISABLED_PopupAccelerators
#else
#define MAYBE_PopupAccelerators PopupAccelerators
#endif
IN_PROC_BROWSER_TEST_F(OmniboxViewTest, MAYBE_PopupAccelerators) {
// Create a popup.
Browser* popup = CreateBrowserForPopup(browser()->profile());
ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(popup));
......@@ -471,19 +478,19 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, PopupAccelerators) {
GetOmniboxViewForBrowser(popup, &omnibox_view));
#endif
// Navigate to https://helloworld.com
ui_test_utils::NavigateToURL(popup, GURL("https://helloworld.com"));
// Set the edit text to "Hello world".
omnibox_view->SetUserText(ASCIIToUTF16("Hello world"));
chrome::FocusLocationBar(popup);
EXPECT_TRUE(omnibox_view->IsSelectAll());
// Try editing the location bar text -- should be disallowed.
ASSERT_NO_FATAL_FAILURE(SendKeyForBrowser(popup, ui::VKEY_S, 0));
EXPECT_EQ(ASCIIToUTF16("https://helloworld.com"), omnibox_view->GetText());
EXPECT_EQ(ASCIIToUTF16("Hello world"), omnibox_view->GetText());
EXPECT_TRUE(omnibox_view->IsSelectAll());
ASSERT_NO_FATAL_FAILURE(
SendKeyForBrowser(popup, ui::VKEY_X, kCtrlOrCmdMask));
EXPECT_EQ(ASCIIToUTF16("https://helloworld.com"), omnibox_view->GetText());
EXPECT_EQ(ASCIIToUTF16("Hello world"), omnibox_view->GetText());
EXPECT_TRUE(omnibox_view->IsSelectAll());
#if !defined(OS_CHROMEOS) && !defined(OS_MACOSX)
......@@ -871,13 +878,13 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, BasicTextOperations) {
ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_B, 0));
ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_C, 0));
// Check if RevertAll() resets the text and resets cursor position
// Check if RevertAll() resets the text and preserves the cursor position.
omnibox_view->RevertAll();
EXPECT_FALSE(omnibox_view->IsSelectAll());
EXPECT_EQ(old_text, omnibox_view->GetText());
omnibox_view->GetSelectionBounds(&start, &end);
EXPECT_EQ(11U, start);
EXPECT_EQ(11U, end);
EXPECT_EQ(3U, start);
EXPECT_EQ(3U, end);
// Check that reverting clamps the cursor to the bounds of the new text.
// Move the cursor to the end.
......
......@@ -205,6 +205,8 @@ AutocompleteMatch OmniboxEditModel::CurrentMatch(
}
bool OmniboxEditModel::ResetDisplayUrls() {
const base::string16 old_current_permanent_url = GetCurrentPermanentUrlText();
url_for_editing_ = controller()->GetToolbarModel()->GetFormattedFullURL();
display_only_url_ =
OmniboxFieldTrial::IsHideSteadyStateUrlSchemeAndSubdomainsEnabled()
......@@ -221,7 +223,8 @@ bool OmniboxEditModel::ResetDisplayUrls() {
// always safe to change the text; this also prevents someone toggling "Show
// URL" (which sounds as if it might be persistent) from seeing just that URL
// forever afterwards.
return !has_focus() || (!user_input_in_progress_ && !PopupIsOpen());
return (GetCurrentPermanentUrlText() != old_current_permanent_url) &&
(!has_focus() || (!user_input_in_progress_ && !PopupIsOpen()));
}
GURL OmniboxEditModel::PermanentURL() const {
......
......@@ -274,9 +274,3 @@ TEST_F(OmniboxEditModelTest, DisablePasteAndGoForLongTexts) {
std::string(OmniboxEditModel::kMaxPasteAndGoTextLength + 1, '.'));
EXPECT_FALSE(model()->OmniboxEditModel::CanPasteAndGo(long_text));
}
// Verify ResetDisplayUrls returns true even if omnibox text has not been
// modified
TEST_F(OmniboxEditModelTest, ResetOmniboxTextOnRenavigatingCurrentPage) {
EXPECT_TRUE(model()->ResetDisplayUrls());
}
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