Commit a3af97f6 authored by Emily Stark's avatar Emily Stark Committed by Commit Bot

Apply simplified domain elisions when omnibox's bounds change

Textfield::OnBoundsChanged refits the RenderText's display rect to the
local bounds. This undoes any simplified domain elisions that have
already been applied. This can happen e.g. when a security chip or
other omnibox decoration appears. In this CL, we override
Textfield::OnBoundsChanged() to reapply any simplified domain elisions
that have previously been applied.

If an animation was in-progress when OnBoundsChanged() runs, we cancel
the animation and abruptly transition to the end state. This might
look a little odd, but it should be quite rare.

Bug: 1102944
Change-Id: Ide5e4a890a5de0e0f092c545dd563106bee9825e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2292456
Commit-Queue: Emily Stark <estark@chromium.org>
Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788908}
parent f831fc29
......@@ -1546,6 +1546,46 @@ bool OmniboxViewViews::HandleAccessibleAction(
return Textfield::HandleAccessibleAction(action_data);
}
void OmniboxViewViews::OnBoundsChanged(const gfx::Rect& previous_bounds) {
Textfield::OnBoundsChanged(previous_bounds);
if (!OmniboxFieldTrial::ShouldRevealPathQueryRefOnHover() &&
!OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction()) {
return;
}
// When simplified domain display field trials are enabled,
// Textfield::OnBoundsChanged() may have undone the effect of any previous URL
// elisions, because it expands the Textfield's display rect to the local
// bounds, which may bring more of the URL into view than intended. Re-apply
// simplified domain elisions now.
// Cancel any running animations. This could cause some abrupt transitions,
// but we can't adapt running animations to new bounds.
if (hover_elide_or_unelide_animation_)
hover_elide_or_unelide_animation_->Stop();
if (elide_after_interaction_animation_)
elide_after_interaction_animation_->Stop();
// |elide_after_interaction_animation_| is created when the user interacts
// with the page, if hide-on-interaction is enabled. If hide-on-interaction is
// disabled or the user has already interacted with the page, the simplified
// domain should have been showing before the bounds changed (or we would have
// been in the process of animating to the simplified domain).
if (!OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction() ||
elide_after_interaction_animation_) {
if (IsURLEligibleForSimplifiedDomainEliding() &&
!model()->ShouldPreventElision()) {
ElideURL();
}
} else {
// The user hasn't interacted with the page yet. This resets animation state
// and shows the partially elided URL with scheme and trivial subdomains
// hidden.
ResetToHideOnInteraction();
}
}
void OmniboxViewViews::OnFocus() {
views::Textfield::OnFocus();
// TODO(tommycli): This does not seem like it should be necessary.
......
......@@ -173,6 +173,10 @@ class OmniboxViewViews : public OmniboxView,
FRIEND_TEST_ALL_PREFIXES(
OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
UserInteractionAndHover);
FRIEND_TEST_ALL_PREFIXES(
OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
BoundsChanged);
FRIEND_TEST_ALL_PREFIXES(OmniboxViewViewsRevealOnHoverTest, BoundsChanged);
FRIEND_TEST_ALL_PREFIXES(
OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
SchemeAndTrivialSubdomainElision);
......@@ -367,6 +371,7 @@ class OmniboxViewViews : public OmniboxView,
bool SkipDefaultKeyEventProcessing(const ui::KeyEvent& event) override;
void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
bool HandleAccessibleAction(const ui::AXActionData& action_data) override;
void OnBoundsChanged(const gfx::Rect& previous_bounds) override;
void OnFocus() override;
void OnBlur() override;
base::string16 GetSelectionClipboardText() const override;
......
......@@ -1738,6 +1738,94 @@ TEST_P(OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
omnibox_view(), kSimplifiedDomainDisplayUrlSubdomainAndScheme));
}
// Tests that simplified domain elisions are re-applied when the omnibox's
// bounds change.
TEST_P(OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest, BoundsChanged) {
SetUpSimplifiedDomainTest();
gfx::RenderText* render_text = omnibox_view()->GetRenderText();
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())));
// After the bounds change, the URL should remain unelided.
omnibox_view()->OnBoundsChanged(gfx::Rect());
ASSERT_NO_FATAL_FAILURE(ExpectUnelidedFromSimplifiedDomain(
omnibox_view()->GetRenderText(),
gfx::Range(kSimplifiedDomainDisplayUrlScheme.size(),
kSimplifiedDomainDisplayUrl.size())));
// Hover over the omnibox and change the bounds during the animation. The
// animation should be cancelled and immediately transition back to the
// unelided URL.
omnibox_view()->OnMouseMoved(CreateMouseEvent(ui::ET_MOUSE_MOVED, {0, 0}));
OmniboxViewViews::ElideAnimation* unelide_animation =
omnibox_view()->GetHoverElideOrUnelideAnimationForTesting();
ASSERT_TRUE(unelide_animation);
EXPECT_TRUE(unelide_animation->IsAnimating());
omnibox_view()->OnBoundsChanged(gfx::Rect());
ASSERT_NO_FATAL_FAILURE(ExpectUnelidedFromSimplifiedDomain(
omnibox_view()->GetRenderText(),
gfx::Range(kSimplifiedDomainDisplayUrlScheme.size(),
kSimplifiedDomainDisplayUrl.size())));
// Simulate a user interaction and change the bounds during the animation. The
// animation should be cancelled and immediately transition to the animation's
// end state (simplified domain).
omnibox_view()->DidGetUserInteraction(
blink::WebInputEvent::Type::kGestureScrollBegin);
OmniboxViewViews::ElideAnimation* elide_animation =
omnibox_view()->GetElideAfterInteractionAnimationForTesting();
ASSERT_TRUE(elide_animation);
EXPECT_TRUE(elide_animation->IsAnimating());
omnibox_view()->OnBoundsChanged(gfx::Rect());
ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
render_text, kSimplifiedDomainDisplayUrlSubdomainAndScheme,
kSimplifiedDomainDisplayUrlSubdomain,
kSimplifiedDomainDisplayUrlHostnameAndScheme,
kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));
}
// Tests that simplified domain elisions are re-applied when the omnibox's
// bounds change when only reveal-on-hover is enabled.
TEST_P(OmniboxViewViewsRevealOnHoverTest, BoundsChanged) {
SetUpSimplifiedDomainTest();
gfx::RenderText* render_text = omnibox_view()->GetRenderText();
ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
render_text, kSimplifiedDomainDisplayUrlSubdomainAndScheme,
kSimplifiedDomainDisplayUrlSubdomain,
kSimplifiedDomainDisplayUrlHostnameAndScheme,
kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));
// After the bounds change, the URL should remain elided.
omnibox_view()->OnBoundsChanged(gfx::Rect());
ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
render_text, kSimplifiedDomainDisplayUrlSubdomainAndScheme,
kSimplifiedDomainDisplayUrlSubdomain,
kSimplifiedDomainDisplayUrlHostnameAndScheme,
kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));
// Hover over the omnibox and change the bounds during the animation. The
// animation should be cancelled and immediately transition back to the
// simplified domain.
omnibox_view()->OnMouseMoved(CreateMouseEvent(ui::ET_MOUSE_MOVED, {0, 0}));
OmniboxViewViews::ElideAnimation* unelide_animation =
omnibox_view()->GetHoverElideOrUnelideAnimationForTesting();
ASSERT_TRUE(unelide_animation);
EXPECT_TRUE(unelide_animation->IsAnimating());
omnibox_view()->OnBoundsChanged(gfx::Rect());
ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
render_text, kSimplifiedDomainDisplayUrlSubdomainAndScheme,
kSimplifiedDomainDisplayUrlSubdomain,
kSimplifiedDomainDisplayUrlHostnameAndScheme,
kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));
}
// Tests scheme and trivial subdomain elision when simplified domain field
// trials are enabled.
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