Skip to content

Commit

Permalink
Simplified domain: Further kill the edit bump
Browse files Browse the repository at this point in the history 
The previous CL in this chain reveals trivial subdomain and scheme
when the URL is unelided on hover. This CL adds a hover animation to
the omnibox even before the user interacts with the page (in the
hide-on-interaction field trial). This allows the user to hover over
the omnibox to bring back the scheme and trivial subdomain even before
the URL has been elided to the simplified domain, thereby minimizing
the potential of having an edit bump before interacting the page as
well as after.

The main change is that we need to create the hover animation
earlier. Previously, we were creating it once the user interacts with
the page; now we create it on page load. Each elide/unelide operation
decides whether to use the full URL, partially elided URL (just scheme
and trivial subdomain hidden), or simplified domain based on whether
the user has interacted with the page yet or not.

Bug: 1101486
Change-Id: Id747929df56becadc34f0cb7a581494f4d90df5f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2291310
Commit-Queue: Emily Stark <estark@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788837}
  • Loading branch information
@estark37
estark37 authored and Commit Bot committed Jul 16, 2020
1 parent 5233421 commit 5b3e904
Show file tree
Hide file tree
Showing 3 changed files with 306 additions and 98 deletions.
179 changes: 124 additions & 55 deletions chrome/browser/ui/views/omnibox/omnibox_view_views.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ void OmniboxViewViews::EmphasizeURLComponents() {
// show just the simplified domain until the user specifically interacts
// with the omnibox by hovering over it.
if (IsURLEligibleForSimplifiedDomainEliding()) {
ElideToSimplifiedDomain();
ElideURL();
} else {
// If the text isn't eligible to be elided to a simplified domain, then
// ensure that as much of it is visible as will fit.
Expand Down Expand Up @@ -700,11 +700,7 @@ void OmniboxViewViews::OnThemeChanged() {
set_placeholder_text_color(dimmed_text_color);

if (!model()->ShouldPreventElision() &&
OmniboxFieldTrial::ShouldRevealPathQueryRefOnHover() &&
!OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction()) {
// When reveal-on-hover is enabled but not hide-on-interaction, create
// the hover elision animation now. When hide-on-interaction is enabled,
// the hover animation is created after the user interacts with each page.
OmniboxFieldTrial::ShouldRevealPathQueryRefOnHover()) {
hover_elide_or_unelide_animation_ =
std::make_unique<ElideAnimation>(this, GetRenderText());
}
Expand Down Expand Up @@ -1166,11 +1162,22 @@ void OmniboxViewViews::OnMouseMoved(const ui::MouseEvent& event) {
if (starting_color == gfx::kPlaceholderColor)
starting_color = SK_ColorTRANSPARENT;
hover_elide_or_unelide_animation_->Stop();
std::vector<gfx::Range> ranges_surrounding_simplified_domain;
GetSimplifiedDomainBounds(&ranges_surrounding_simplified_domain);
// Figure out where we are uneliding from so that the hover animation can
// fade the surrounding text in. If the user has already interacted with the
// page, then we elided to the simplified domain and that is what we are
// uneliding from now. Otherwise, only the scheme and possibly a trivial
// subdomain have been elided and those components now need to be faded in.
std::vector<gfx::Range> ranges_to_fade_in;
if (elide_after_interaction_animation_ ||
!OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction()) {
GetSimplifiedDomainBounds(&ranges_to_fade_in);
} else {
url::Component host = GetHostComponentAfterTrivialSubdomain();
ranges_to_fade_in.emplace_back(0, host.begin);
}
hover_elide_or_unelide_animation_->Start(
unelide_bounds, OmniboxFieldTrial::UnelideURLOnHoverThresholdMs(),
ranges_surrounding_simplified_domain, starting_color,
ranges_to_fade_in, starting_color,
GetOmniboxColor(GetThemeProvider(),
OmniboxPart::LOCATION_BAR_TEXT_DIMMED));
}
Expand All @@ -1192,30 +1199,37 @@ void OmniboxViewViews::OnMouseExited(const ui::MouseEvent& event) {
// when their mouse exits the omnibox area. The elision animation is the
// reverse of the unelision animation: we shrink the URL from both sides while
// shifting the text to the leading edge.

// When hide-on-interaction is enabled, we don't want to elide or unelide
// until there's user interaction with the page. In this variation,
// |hover_elide_or_unelide_animation_| is created in DidGetUserInteraction()
// so its existence signals that user interaction has taken place already.
if (hover_elide_or_unelide_animation_) {
SkColor starting_color =
hover_elide_or_unelide_animation_->IsAnimating()
? hover_elide_or_unelide_animation_->GetCurrentColor()
: GetOmniboxColor(GetThemeProvider(),
OmniboxPart::LOCATION_BAR_TEXT_DIMMED);
hover_elide_or_unelide_animation_->Stop();
// Elisions don't take display offset into account (see
// https://crbug.com/1099078), so the RenderText must be in NO_ELIDE mode to
// avoid over-eliding when some of the text is not visible due to display
// offset.
GetRenderText()->SetElideBehavior(gfx::NO_ELIDE);
DCHECK(hover_elide_or_unelide_animation_);
SkColor starting_color =
hover_elide_or_unelide_animation_->IsAnimating()
? hover_elide_or_unelide_animation_->GetCurrentColor()
: GetOmniboxColor(GetThemeProvider(),
OmniboxPart::LOCATION_BAR_TEXT_DIMMED);
hover_elide_or_unelide_animation_->Stop();
// Elisions don't take display offset into account (see
// https://crbug.com/1099078), so the RenderText must be in NO_ELIDE mode to
// avoid over-eliding when some of the text is not visible due to display
// offset.
GetRenderText()->SetElideBehavior(gfx::NO_ELIDE);
// Figure out where to elide to. If the user has already interacted with the
// page or reveal-on-interaction is disabled, then elide to the simplified
// domain; otherwise just hide the scheme and trivial subdomain (if any).
if (elide_after_interaction_animation_ ||
!OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction()) {
std::vector<gfx::Range> ranges_surrounding_simplified_domain;
gfx::Range simplified_domain =
GetSimplifiedDomainBounds(&ranges_surrounding_simplified_domain);
hover_elide_or_unelide_animation_->Start(
simplified_domain, 0 /* delay_ms */,
ranges_surrounding_simplified_domain, starting_color,
SK_ColorTRANSPARENT);
} else {
base::string16 text = GetText();
url::Component host = GetHostComponentAfterTrivialSubdomain();
hover_elide_or_unelide_animation_->Start(
gfx::Range(host.begin, text.size()), 0 /* delay_ms */,
std::vector<gfx::Range>{gfx::Range(0, host.begin)}, starting_color,
SK_ColorTRANSPARENT);
}
}

Expand Down Expand Up @@ -1548,7 +1562,7 @@ void OmniboxViewViews::OnFocus() {
saved_selection_for_focus_change_.clear();
}

UnelideFromSimplifiedDomain();
ShowFullURL();
GetRenderText()->SetElideBehavior(gfx::NO_ELIDE);

// Focus changes can affect the visibility of any keyword hint.
Expand Down Expand Up @@ -1673,7 +1687,7 @@ void OmniboxViewViews::OnBlur() {
std::make_unique<OmniboxViewViews::ElideAnimation>(this,
GetRenderText());
if (IsURLEligibleForSimplifiedDomainEliding()) {
ElideToSimplifiedDomain();
ElideURL();
} else {
// If the text isn't eligible to be elided to a simplified domain, then
// ensure that as much of it is visible as will fit.
Expand Down Expand Up @@ -1734,7 +1748,7 @@ void OmniboxViewViews::DidFinishNavigation(
if (IsURLEligibleForSimplifiedDomainEliding() &&
elide_after_interaction_animation_ &&
!elide_after_interaction_animation_->IsAnimating()) {
ElideToSimplifiedDomain();
ElideURL();
}
return;
}
Expand All @@ -1750,6 +1764,11 @@ void OmniboxViewViews::DidGetUserInteraction(
return;
}

// If there's already a hover animation running, just let it run as we will
// end up at the same place.
if (hover_elide_or_unelide_animation_->IsAnimating())
return;

// This method runs when the user interacts with the page, such as scrolling
// or typing. In the hide-on-interaction field trial, the URL is shown until
// user interaction, at which point it's animated to a simplified version of
Expand All @@ -1774,14 +1793,6 @@ void OmniboxViewViews::DidGetUserInteraction(
OmniboxPart::LOCATION_BAR_TEXT_DIMMED),
SK_ColorTRANSPARENT);
}
// Now that the URL is being elided, create the animation to bring it back on
// hover (if enabled via field trial), if it hasn't already been created on an
// earlier call to this method.
if (OmniboxFieldTrial::ShouldRevealPathQueryRefOnHover() &&
!hover_elide_or_unelide_animation_) {
hover_elide_or_unelide_animation_ =
std::make_unique<ElideAnimation>(this, GetRenderText());
}
}

base::string16 OmniboxViewViews::GetSelectionClipboardText() const {
Expand Down Expand Up @@ -2231,8 +2242,7 @@ gfx::Range OmniboxViewViews::GetSimplifiedDomainBounds(
base::string16 text = GetText();
url::Component host = GetHostComponentAfterTrivialSubdomain();
if (ranges_surrounding_simplified_domain) {
ranges_surrounding_simplified_domain->push_back(
gfx::Range(host.end(), text.size()));
ranges_surrounding_simplified_domain->emplace_back(host.end(), text.size());
}

if (!OmniboxFieldTrial::ShouldElideToRegistrableDomain()) {
Expand All @@ -2257,8 +2267,8 @@ gfx::Range OmniboxViewViews::GetSimplifiedDomainBounds(
text.find(base::ASCIIToUTF16(simplified_domain));
DCHECK_NE(simplified_domain_pos, std::string::npos);
if (ranges_surrounding_simplified_domain) {
ranges_surrounding_simplified_domain->push_back(
gfx::Range(0, simplified_domain_pos));
ranges_surrounding_simplified_domain->emplace_back(0,
simplified_domain_pos);
}
return gfx::Range(simplified_domain_pos, host.end());
}
Expand Down Expand Up @@ -2287,13 +2297,15 @@ void OmniboxViewViews::ResetToHideOnInteraction() {
model()->ShouldPreventElision()) {
return;
}
// Delete the animations; they'll get recreated in DidGetUserInteraction().
// This prevents us from running any animations until the user interacts with
// the page.
hover_elide_or_unelide_animation_.reset();
// Delete the interaction animation; it'll get recreated in
// DidGetUserInteraction(). Recreate the hover animation now because the user
// can hover over the URL before interacting with the page to reveal the
// scheme and trivial subdomain (if any).
elide_after_interaction_animation_.reset();
hover_elide_or_unelide_animation_ =
std::make_unique<OmniboxViewViews::ElideAnimation>(this, GetRenderText());
if (IsURLEligibleForSimplifiedDomainEliding())
UnelideFromSimplifiedDomain();
ShowFullURLWithoutSchemeAndTrivialSubdomain();
}

void OmniboxViewViews::OnShouldPreventElisionChanged() {
Expand All @@ -2306,7 +2318,7 @@ void OmniboxViewViews::OnShouldPreventElisionChanged() {
hover_elide_or_unelide_animation_.reset();
elide_after_interaction_animation_.reset();
if (IsURLEligibleForSimplifiedDomainEliding())
UnelideFromSimplifiedDomain();
ShowFullURL();
return;
}
if (OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction()) {
Expand All @@ -2315,19 +2327,16 @@ void OmniboxViewViews::OnShouldPreventElisionChanged() {
ResetToHideOnInteraction();
} else if (OmniboxFieldTrial::ShouldRevealPathQueryRefOnHover()) {
if (IsURLEligibleForSimplifiedDomainEliding()) {
ElideToSimplifiedDomain();
ElideURL();
}
hover_elide_or_unelide_animation_ =
std::make_unique<ElideAnimation>(this, GetRenderText());
}
}

void OmniboxViewViews::ElideToSimplifiedDomain() {
if (!OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction() &&
!OmniboxFieldTrial::ShouldRevealPathQueryRefOnHover()) {
return;
}

void OmniboxViewViews::ElideURL() {
DCHECK(OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction() ||
OmniboxFieldTrial::ShouldRevealPathQueryRefOnHover());
DCHECK(IsURLEligibleForSimplifiedDomainEliding());

// The simplified domain string must be a substring of the current display
Expand Down Expand Up @@ -2370,7 +2379,7 @@ void OmniboxViewViews::ElideToSimplifiedDomain() {
(simplified_domain_rect.x() - old_bounds.x()));
}

void OmniboxViewViews::UnelideFromSimplifiedDomain() {
void OmniboxViewViews::ShowFullURL() {
if (!OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction() &&
!OmniboxFieldTrial::ShouldRevealPathQueryRefOnHover()) {
return;
Expand All @@ -2385,6 +2394,66 @@ void OmniboxViewViews::UnelideFromSimplifiedDomain() {
GetRenderText()->SetElideBehavior(gfx::ELIDE_TAIL);
}

void OmniboxViewViews::ShowFullURLWithoutSchemeAndTrivialSubdomain() {
DCHECK(IsURLEligibleForSimplifiedDomainEliding());
DCHECK(OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction() ||
OmniboxFieldTrial::ShouldRevealPathQueryRefOnHover());
DCHECK(!model()->ShouldPreventElision());

// First show the full URL, then figure out what to elide.
ShowFullURL();

if (!IsURLEligibleForSimplifiedDomainEliding() ||
model()->ShouldPreventElision()) {
return;
}

// TODO(https://crbug.com/1099078): currently, we cannot set the elide
// behavior to anything other than NO_ELIDE when the display offset is 0, i.e.
// when we are not hiding the scheme and trivial subdomain. This is because
// RenderText does not take display offset into account when eliding, so it
// will over-elide by however much text is scrolled out of the display area.
GetRenderText()->SetElideBehavior(gfx::NO_ELIDE);

GetRenderText()->SetDisplayOffset(0);
const gfx::Rect& current_display_rect = GetRenderText()->display_rect();

// If the scheme and trivial subdomain should be elided, then we want to set
// the display offset to where the hostname after the trivial subdomain (if
// any) begins, relative to the current display rect.
base::string16 text = GetText();
url::Component host = GetHostComponentAfterTrivialSubdomain();

gfx::Rect display_url_bounds;
for (const auto& rect : GetRenderText()->GetSubstringBounds(
gfx::Range(host.begin, text.size()))) {
display_url_bounds.Union(rect);
}
display_url_bounds.set_height(current_display_rect.height());
display_url_bounds.set_y(current_display_rect.y());

// Set the scheme and trivial subdomain to transparent. This isn't necessary
// to hide this portion of the text because it will be scrolled out of
// visibility anyway when we set the display offset below. However, if the
// user subsequently hovers over the URL to bring back the scheme and trivial
// subdomain, the hover animation assumes that the hidden text starts from
// transparent and fades it back in.
ApplyColor(SK_ColorTRANSPARENT, gfx::Range(0, host.begin));

// Before setting the display offset, set the display rect to the portion of
// the URL that won't be elided, or leave it at the local bounds, whichever is
// smaller. The display offset is capped at 0 if the text doesn't overflow the
// display rect, so we must fit the display rect to the text so that we can
// then set the display offset to scroll the scheme and trivial subdomain out
// of visibility.
GetRenderText()->SetDisplayRect(
gfx::Rect(current_display_rect.x(), display_url_bounds.y(),
display_url_bounds.width(), display_url_bounds.height()));

GetRenderText()->SetDisplayOffset(
-1 * (display_url_bounds.x() - current_display_rect.x()));
}

url::Component OmniboxViewViews::GetHostComponentAfterTrivialSubdomain() {
url::Component host;
url::Component unused_scheme;
Expand Down
49 changes: 28 additions & 21 deletions chrome/browser/ui/views/omnibox/omnibox_view_views.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,9 @@ class OmniboxViewViews : public OmniboxView,
FRIEND_TEST_ALL_PREFIXES(
OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
UserInteractionAndHover);
FRIEND_TEST_ALL_PREFIXES(
OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
SchemeAndTrivialSubdomainElision);
FRIEND_TEST_ALL_PREFIXES(
OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
HideOnInteractionAfterFocusAndBlur);
Expand Down Expand Up @@ -447,16 +450,27 @@ class OmniboxViewViews : public OmniboxView,
// display.
void OnShouldPreventElisionChanged();

// Elides or unelides to a simplified version of the URL. Callers should
// ensure that the URL is valid before calling.
// The methods below elide to or unelide from a simplified version of the URL.
// Callers should ensure that the URL is valid before calling.
//
// These methods do not animate, but rather immediately elide/unelide. These
// methods are used when we don't want to draw the user's attention to the URL
// simplification -- for example, if the URL is already simplified and the
// user performs a same-document navigation, we want to keep the URL
// simplified without it appearing to be a change from the user's perspective.
void ElideToSimplifiedDomain();
void UnelideFromSimplifiedDomain();

// Elides the URL to a simplified version of the domain. This will be the
// registrable domain if OmniboxFieldTrial::ShouldElideToRegistrableDomain()
// is true; otherwise it is the hostname with trivial subdomains ("www.")
// elided. The scheme, path, and other components of the URL are hidden.
void ElideURL();
// Show the full URL, including scheme, all subdomains, and path.
void ShowFullURL();
// Shows the full URL and then elides http/https schemes and the
// "www." subdomain (if present) by setting the display rect to the width of
// the remaining URL and then setting the display offset to scroll the scheme
// and trivial subdomain offscreen.
void ShowFullURLWithoutSchemeAndTrivialSubdomain();

// Parses GetText() as a URL, trims trivial subdomains from it (if any and if
// applicable), and returns the result.
Expand All @@ -483,26 +497,19 @@ class OmniboxViewViews : public OmniboxView,
//
// These animations are used by different field trials as described below.

// When ShouldRevealPathQueryRefOnHover() is enabled but not
// ShouldHidePathQueryRefOnInteraction(), then the URL is elided in
// EmphasizeUrlComponents() and |hover_elide_or_unelide_animation_| is created
// in OnThemeChanged(). This animation is used to unelide or elide the URL
// when the mouse hovers or exits the omnibox.
// This animation is used to unelide or elide the URL
// when the mouse hovers or exits the omnibox. The URL will unelide to the
// full URL or a partially elided version (with scheme and trivial subdomains
// elided) depending on whether the user has interacted with the page yet
// (when reveal-on-interaction is enabled).
std::unique_ptr<ElideAnimation> hover_elide_or_unelide_animation_;
// When ShouldHidePathQueryRefOnInteraction() is enabled, we don't
// create any animations until the user interacts with the page. When a
// When ShouldHidePathQueryRefOnInteraction() is enabled, when a
// navigation finishes, we unelide the URL if it was a full cross-document
// navigation. Once the user interacts with the page, we create and run
// |elide_after_interaction_animation_| to elide the URL. If
// ShouldRevealPathQueryRefOnHover() is also enabled, we defer the creation of
// |hover_elide_or_unelide_animation_| until the user interacts with the page
// as well, since we don't want to do any hover animations until the URL has
// been elided after user interaction. After the first user interaction,
// |elide_after_interaction_animation_| doesn't run again until it's
// re-created after the next navigation, and
// |hover_elide_or_unelide_animation_| behaves as described above for the rest
// of the navigation. There are 2 separate animations (one for
// after-interaction and one hovering) so that the state of the
// |elide_after_interaction_animation_| to elide the URL. After the first user
// interaction, |elide_after_interaction_animation_| doesn't run again until
// it's re-created after the next navigation. There are 2 separate animations
// (one for after-interaction and one hovering) so that the state of the
// after-interaction animation can be queried to know when the user has or has
// not already interacted with the page.
std::unique_ptr<ElideAnimation> elide_after_interaction_animation_;
Expand Down

0 comments on commit 5b3e904

Please sign in to comment.