Commit b7e58d9d authored by manuk's avatar manuk Committed by Commit Bot

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