Commit 8ea67049 authored by Emily Stark's avatar Emily Stark Committed by Commit Bot

Restore omnibox bounds when resetting if not eligible for eliding

This CL fixes a bug in the code that resets to the on-page-load state
for the simplified domain hide-on-interaction field trial.

The bug is that this reset code let the omnibox bounds go unchanged
if the new text was not eligible for simplified domain eliding.
For example, when switching to a new tab (as shown in
https://bugs.chromium.org/p/chromium/issues/detail?id=1112577#c4), the
placeholder text could get cut off because the omnibox was remaining
at the bounds used for the last simplified domain elision. The fix is
to restore the omnibox to its local bounds, with ELIDE_TAIL behavior,
when the text is not eligible for simplified domain eliding.

Bug: 1107912, 1112577
Change-Id: I36c359831fc1f7b18c814d9144b801655bd3b1c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2342123Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795953}
parent 95a10dec
...@@ -2446,8 +2446,12 @@ void OmniboxViewViews::ResetToHideOnInteraction() { ...@@ -2446,8 +2446,12 @@ void OmniboxViewViews::ResetToHideOnInteraction() {
elide_after_web_contents_interaction_animation_.reset(); elide_after_web_contents_interaction_animation_.reset();
hover_elide_or_unelide_animation_ = hover_elide_or_unelide_animation_ =
std::make_unique<OmniboxViewViews::ElideAnimation>(this, GetRenderText()); std::make_unique<OmniboxViewViews::ElideAnimation>(this, GetRenderText());
if (IsURLEligibleForSimplifiedDomainEliding()) if (IsURLEligibleForSimplifiedDomainEliding()) {
ShowFullURLWithoutSchemeAndTrivialSubdomain(); ShowFullURLWithoutSchemeAndTrivialSubdomain();
} else {
GetRenderText()->SetElideBehavior(gfx::ELIDE_TAIL);
FitToLocalBounds();
}
} }
void OmniboxViewViews::OnShouldPreventElisionChanged() { void OmniboxViewViews::OnShouldPreventElisionChanged() {
......
...@@ -220,6 +220,9 @@ class OmniboxViewViews : public OmniboxView, ...@@ -220,6 +220,9 @@ class OmniboxViewViews : public OmniboxView,
FRIEND_TEST_ALL_PREFIXES( FRIEND_TEST_ALL_PREFIXES(
OmniboxViewViewsRevealOnHoverAndMaybeHideOnInteractionTest, OmniboxViewViewsRevealOnHoverAndMaybeHideOnInteractionTest,
UnsetAlwaysShowFullURLs); UnsetAlwaysShowFullURLs);
FRIEND_TEST_ALL_PREFIXES(
OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
TabChangeWhenNotEligibleForEliding);
FRIEND_TEST_ALL_PREFIXES(OmniboxPopupContentsViewTest, FRIEND_TEST_ALL_PREFIXES(OmniboxPopupContentsViewTest,
EmitAccessibilityEvents); EmitAccessibilityEvents);
// TODO(tommycli): Remove the rest of these friends after porting these // TODO(tommycli): Remove the rest of these friends after porting these
......
...@@ -2290,6 +2290,51 @@ TEST_P(OmniboxViewViewsRevealOnHoverAndMaybeHideOnInteractionTest, ...@@ -2290,6 +2290,51 @@ TEST_P(OmniboxViewViewsRevealOnHoverAndMaybeHideOnInteractionTest,
EXPECT_TRUE(elide_animation->IsAnimating()); EXPECT_TRUE(elide_animation->IsAnimating());
} }
// Tests that in the hide-on-interaction field trial, the omnibox is reset to
// the local bounds on tab change when the new text is not eligible for
// simplified domain elision.
TEST_P(OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
TabChangeWhenNotEligibleForEliding) {
SetUpSimplifiedDomainTest();
content::MockNavigationHandle navigation;
navigation.set_is_same_document(false);
omnibox_view()->DidFinishNavigation(&navigation);
ASSERT_NO_FATAL_FAILURE(ExpectUnelidedFromSimplifiedDomain(
omnibox_view()->GetRenderText(),
gfx::Range(kSimplifiedDomainDisplayUrlScheme.size(),
kSimplifiedDomainDisplayUrl.size())));
// Simulate a user interaction and advance through the animation to elide the
// URL.
omnibox_view()->DidGetUserInteraction(blink::WebKeyboardEvent());
OmniboxViewViews::ElideAnimation* elide_animation =
omnibox_view()->GetElideAfterInteractionAnimationForTesting();
gfx::AnimationContainerElement* elide_as_element =
elide_animation->GetAnimationForTesting();
elide_as_element->SetStartTime(base::TimeTicks());
elide_as_element->Step(base::TimeTicks() + base::TimeDelta::FromSeconds(1));
ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
omnibox_view(), kSimplifiedDomainDisplayUrlScheme,
kSimplifiedDomainDisplayUrlSubdomain,
kSimplifiedDomainDisplayUrlHostnameAndScheme,
kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));
// Change the tab and set state such that the current text is not eligible for
// simplified domain eliding. The omnibox should take up the full local bounds
// and be reset to tail-eliding behavior, just as if the above simplified
// domain elision had not happened.
omnibox_view()->model()->SetInputInProgress(true);
std::unique_ptr<content::WebContents> web_contents =
content::WebContentsTester::CreateTestWebContents(profile(), nullptr);
omnibox_view()->SaveStateToTab(web_contents.get());
omnibox_view()->OnTabChanged(web_contents.get());
EXPECT_EQ(gfx::ELIDE_TAIL, omnibox_view()->GetRenderText()->elide_behavior());
EXPECT_EQ(kSimplifiedDomainDisplayUrl,
omnibox_view()->GetRenderText()->GetDisplayText());
}
// Tests that in the hide-on-interaction field trial, when the path changes // Tests that in the hide-on-interaction field trial, when the path changes
// while being elided, the animation is stopped. // while being elided, the animation is stopped.
TEST_P(OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest, TEST_P(OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
......
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