Add omnibox variation that shows path until interaction
This implements the flag
omnibox-ui-hide-steady-state-url-path-query-and-ref-on-interaction. In
this variation, the path is shown on page load until the user
interacts with the page.
Bug: 1090393
Change-Id: Ie21b67a0939dbc7938e00ba477c2c7abab59b61b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2234128
Commit-Queue: Emily Stark <estark@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#776357}
diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
index f5f5b67..136a606e 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
+++ b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
@@ -288,6 +288,8 @@
// Initialize the popup view using the same font.
popup_view_.reset(
new OmniboxPopupContentsView(this, model(), location_bar_view_));
+ if (OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction())
+ Observe(location_bar_view_->GetWebContents());
}
// Override the default FocusableBorder from Textfield, since the
@@ -404,8 +406,19 @@
path_fade_out_animation_->Start(GetPathBounds());
}
- if (path_fade_in_animation_ && CanFadePath()) {
- ApplyColor(SK_ColorTRANSPARENT, GetPathBounds());
+ if (OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction()) {
+ Observe(location_bar_view_->GetWebContents());
+ // Clear the fade-in animation (if it exists; it won't exist if
+ // reveal-on-hover is not enabled). If reveal-on-hover is enabled, it will
+ // be recreated after the path is faded out in DidGetUserInteraction().
+ // Creating it here would cause the path to unnecessarily fade in on hover
+ // before it's been faded out by user interaction.
+ path_fade_in_animation_.reset();
+ } else if (OmniboxFieldTrial::ShouldRevealPathQueryRefOnHover()) {
+ // If reveal-on-hover is enabled and hide-on-interaction is disabled, hide
+ // the path now.
+ if (CanFadePath())
+ ApplyColor(SK_ColorTRANSPARENT, GetPathBounds());
}
}
@@ -640,17 +653,25 @@
// In on-hover and on-interaction variations, the path fades in or out based
// on user interactions, not automatically after a timeout.
- if (!OmniboxFieldTrial::ShouldRevealPathQueryRefOnHover() &&
- !OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction()) {
- path_fade_out_animation_ = std::make_unique<PathFadeAnimation>(
- this, dimmed_text_color, SK_ColorTRANSPARENT, kPathFadeOutDelayMs);
- }
- if (OmniboxFieldTrial::ShouldRevealPathQueryRefOnHover()) {
+ if (OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction()) {
+ // When hiding the path on interaction, don't create the fade-in animation
+ // yet. The hover fade-in animation (if enabled) will be created later in
+ // DidGetUserInteraction() after the path is faded out.
+ path_fade_out_fast_animation_ = std::make_unique<PathFadeAnimation>(
+ this, dimmed_text_color, SK_ColorTRANSPARENT, 0);
+ } else if (OmniboxFieldTrial::ShouldRevealPathQueryRefOnHover()) {
+ // When reveal-on-hover is enabled but not hide-on-interaction, create
+ // both the fade-in and fade-out animations now.
path_fade_in_animation_ = std::make_unique<PathFadeAnimation>(
this, SK_ColorTRANSPARENT, dimmed_text_color,
kExtendedHoverThresholdMs);
path_fade_out_fast_animation_ = std::make_unique<PathFadeAnimation>(
this, dimmed_text_color, SK_ColorTRANSPARENT, 0);
+ } else {
+ // When neither reveal-on-hover nor hide-on-interaction are enabled, fade
+ // out the path after a fixed delay.
+ path_fade_out_animation_ = std::make_unique<PathFadeAnimation>(
+ this, dimmed_text_color, SK_ColorTRANSPARENT, kPathFadeOutDelayMs);
}
}
@@ -717,7 +738,7 @@
return views::Textfield::GetSelectedText();
}
-void OmniboxViewViews::OnPaste() {
+void OmniboxViewViews::OnOmniboxPaste() {
const base::string16 text(GetClipboardText());
if (text.empty() ||
@@ -1144,7 +1165,7 @@
if (!CanFadePath())
return;
path_fade_out_fast_animation_->Stop();
- if (!path_fade_in_animation_->IsAnimating())
+ if (path_fade_in_animation_ && !path_fade_in_animation_->IsAnimating())
path_fade_in_animation_->Start(GetPathBounds());
}
@@ -1158,10 +1179,16 @@
}
if (!CanFadePath())
return;
- path_fade_out_fast_animation_->ResetStartingColor(
- path_fade_in_animation_->GetCurrentColor());
- path_fade_in_animation_->Stop();
- path_fade_out_fast_animation_->Start(GetPathBounds());
+ // When hide-on-interaction is enabled, we don't want to fade the path in or
+ // out until there's user interaction with the page. In this variation,
+ // |path_fade_in_animation_| is created in DidGetUserInteraction() so its
+ // existence signals that user interaction has taken place already.
+ if (path_fade_in_animation_) {
+ path_fade_out_fast_animation_->ResetStartingColor(
+ path_fade_in_animation_->GetCurrentColor());
+ path_fade_in_animation_->Stop();
+ path_fade_out_fast_animation_->Start(GetPathBounds());
+ }
}
bool OmniboxViewViews::IsItemForCommandIdDynamic(int command_id) const {
@@ -1586,6 +1613,27 @@
location_bar_view_->command_updater()->IsCommandEnabled(command_id);
}
+void OmniboxViewViews::DidGetUserInteraction(
+ const blink::WebInputEvent::Type type) {
+ if (!OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction())
+ return;
+
+ DCHECK(path_fade_out_fast_animation_);
+ path_fade_out_fast_animation_->Stop();
+ if (CanFadePath())
+ path_fade_out_fast_animation_->Start(GetPathBounds());
+ // Now that the path is fading out, create the animation to bring it back on
+ // hover (if enabled via field trial).
+ if (OmniboxFieldTrial::ShouldRevealPathQueryRefOnHover()) {
+ path_fade_in_animation_ = std::make_unique<PathFadeAnimation>(
+ this, SK_ColorTRANSPARENT,
+ GetOmniboxColor(GetThemeProvider(),
+ OmniboxPart::LOCATION_BAR_TEXT_DIMMED),
+ kExtendedHoverThresholdMs);
+ }
+ Observe(nullptr);
+}
+
base::string16 OmniboxViewViews::GetSelectionClipboardText() const {
return SanitizeTextForPaste(Textfield::GetSelectionClipboardText());
}
@@ -1639,7 +1687,7 @@
model()->OnUpOrDownKeyPressed(1);
break;
case ui::TextEditCommand::PASTE:
- OnPaste();
+ OnOmniboxPaste();
break;
default:
Textfield::ExecuteTextEditCommand(command);
diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_views.h b/chrome/browser/ui/views/omnibox/omnibox_view_views.h
index b8dca62..49d72594b 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_view_views.h
+++ b/chrome/browser/ui/views/omnibox/omnibox_view_views.h
@@ -20,6 +20,7 @@
#include "components/prefs/pref_change_registrar.h"
#include "components/search_engines/template_url_service.h"
#include "components/search_engines/template_url_service_observer.h"
+#include "content/public/browser/web_contents_observer.h"
#include "ui/base/window_open_disposition.h"
#include "ui/compositor/compositor.h"
#include "ui/compositor/compositor_observer.h"
@@ -56,7 +57,8 @@
#endif
public views::TextfieldController,
public ui::CompositorObserver,
- public TemplateURLServiceObserver {
+ public TemplateURLServiceObserver,
+ public content::WebContentsObserver {
public:
// The internal view class name.
static const char kViewClassName[];
@@ -152,6 +154,9 @@
base::string16 GetLabelForCommandId(int command_id) const override;
bool IsCommandIdEnabled(int command_id) const override;
+ // content::WebContentsObserver:
+ void DidGetUserInteraction(const blink::WebInputEvent::Type type) override;
+
// For testing only.
OmniboxPopupContentsView* GetPopupContentsViewForTesting() const {
return popup_view_.get();
@@ -195,7 +200,7 @@
// as is. We want to strip whitespace and other things (see GetClipboardText()
// for details). The function invokes OnBefore/AfterPossibleChange() as
// necessary.
- void OnPaste();
+ void OnOmniboxPaste();
// Handle keyword hint tab-to-search and tabbing through dropdown results.
bool HandleEarlyTabActions(const ui::KeyEvent& event);
@@ -323,7 +328,11 @@
std::unique_ptr<OmniboxPopupContentsView> popup_view_;
- // Animation used to fade in/out the path under some elision settings.
+ // Animations used to fade in/out the path under some elision settings.
+
+ // Fades the path in after a short delay. Under certain variations, this
+ // animation is not created until the user interacts with the page, so it's
+ // not always guaranteed to exist.
std::unique_ptr<PathFadeAnimation> path_fade_in_animation_;
// Waits a few seconds and then fades the path out.
std::unique_ptr<PathFadeAnimation> path_fade_out_animation_;
