Commit 56db3421 authored by Emily Stark's avatar Emily Stark Committed by Commit Bot

Revert "Restore omnibox bounds when resetting if not eligible for eliding"

This reverts commit 8ea67049.

Reason for revert: I noticed that this change causes the omnibox to be in ELIDE_TAIL mode during editing, which causes a DCHECK failure.

Original change's description:
> 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/+/2342123
> Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
> Commit-Queue: Emily Stark <estark@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#795953}

TBR=jdonnelly@chromium.org,estark@chromium.org

Change-Id: Id44cd8e39064af890399109fcca8ca729ddba5d4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1107912
Bug: 1112577
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2343153Reviewed-by: default avatarEmily Stark <estark@chromium.org>
Commit-Queue: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796133}
parent 6cf20e78
......@@ -2455,12 +2455,8 @@ void OmniboxViewViews::ResetToHideOnInteraction() {
elide_after_web_contents_interaction_animation_.reset();
hover_elide_or_unelide_animation_ =
std::make_unique<OmniboxViewViews::ElideAnimation>(this, GetRenderText());
if (IsURLEligibleForSimplifiedDomainEliding()) {
if (IsURLEligibleForSimplifiedDomainEliding())
ShowFullURLWithoutSchemeAndTrivialSubdomain();
} else {
GetRenderText()->SetElideBehavior(gfx::ELIDE_TAIL);
FitToLocalBounds();
}
}
void OmniboxViewViews::OnShouldPreventElisionChanged() {
......
......@@ -225,9 +225,6 @@ class OmniboxViewViews : public OmniboxView,
FRIEND_TEST_ALL_PREFIXES(
OmniboxViewViewsRevealOnHoverAndMaybeHideOnInteractionTest,
UnsetAlwaysShowFullURLs);
FRIEND_TEST_ALL_PREFIXES(
OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
TabChangeWhenNotEligibleForEliding);
FRIEND_TEST_ALL_PREFIXES(OmniboxPopupContentsViewTest,
EmitAccessibilityEvents);
// TODO(tommycli): Remove the rest of these friends after porting these
......
......@@ -2290,51 +2290,6 @@ TEST_P(OmniboxViewViewsRevealOnHoverAndMaybeHideOnInteractionTest,
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
// while being elided, the animation is stopped.
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