Simplified domain: treat focusing editable nodes as user interaction

In some variants of the simplified domain field trials, we show the
full URL on page load and elide to a simplified version of the domain
after the user interacts with the page. Originally we considered any
user interaction (as received via
WebContentsObserver::DidGetUserInteraction) a trigger for elision, but
then started ignoring mouse clicks in
https://chromium-review.googlesource.com/c/chromium/src/+/2327289, to
avoid flashing when the user clicks on links. This proved to be a bit
too coarse, since we want to at least elide the URL when the user
focuses a form text field. This CL thus elides the URL when
WebContentsObserver::OnFocusChangedInPage() reports that an editable
node has been focused.

Bug: 1104280
Change-Id: Ib0b4a5b19d035c38a2e529c6f526c421f537cff4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2357074Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798261}

chrome/browser/ui/views/omnibox/omnibox_view_views.cc

@@ -54,6 +54,7 @@
 #include "components/vector_icons/vector_icons.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/focused_node_details.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
 #include "extensions/common/constants.h"
@@ -1960,11 +1961,6 @@ void OmniboxViewViews::DidFinishNavigation(
 
 void OmniboxViewViews::DidGetUserInteraction(
     const blink::WebInputEvent& event) {
-  if (!OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction() ||
-      model()->ShouldPreventElision()) {
-    return;
-  }
-
   // Exclude mouse clicks from triggering the simplified domain elision. Mouse
   // clicks can be done idly and aren't a good signal of real intent to interact
   // with the page. Plus, it can be jarring when the URL elides when the user
@@ -1980,35 +1976,13 @@ 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
-  // the domain (hiding the path and, optionally, subdomains). The animation is
-  // designed to draw the user's attention and suggest that they can return to
-  // the omnibox to uncover the full URL.
+  MaybeElideURLWithAnimationFromInteraction();
+}
 
-  // Only create and run the animation if we haven't already done so on an
-  // earlier call to this method.
-  if (IsURLEligibleForSimplifiedDomainEliding() &&
-      !elide_after_web_contents_interaction_animation_) {
-    GetRenderText()->SetElideBehavior(gfx::NO_ELIDE);
-    elide_after_web_contents_interaction_animation_ =
-        std::make_unique<ElideAnimation>(this, GetRenderText());
-    std::vector<gfx::Range> ranges_surrounding_simplified_domain;
-    gfx::Range simplified_domain =
-        GetSimplifiedDomainBounds(&ranges_surrounding_simplified_domain);
-    elide_after_web_contents_interaction_animation_->Start(
-        simplified_domain, 0 /* delay_ms */,
-        ranges_surrounding_simplified_domain,
-        GetOmniboxColor(GetThemeProvider(),
-                        OmniboxPart::LOCATION_BAR_TEXT_DIMMED),
-        SK_ColorTRANSPARENT);
-  }
+void OmniboxViewViews::OnFocusChangedInPage(
+    content::FocusedNodeDetails* details) {
+  if (details->is_editable_node)
+    MaybeElideURLWithAnimationFromInteraction();
 }
 
 base::string16 OmniboxViewViews::GetSelectionClipboardText() const {
@@ -2560,6 +2534,43 @@ void OmniboxViewViews::OnShouldPreventElisionChanged() {
   }
 }
 
+void OmniboxViewViews::MaybeElideURLWithAnimationFromInteraction() {
+  if (!OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction() ||
+      model()->ShouldPreventElision()) {
+    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
+  // the domain (hiding the path and, optionally, subdomains). The animation is
+  // designed to draw the user's attention and suggest that they can return to
+  // the omnibox to uncover the full URL.
+
+  // If we've already created and run the animation in an earlier call to this
+  // method, we don't need to do so again.
+  if (!IsURLEligibleForSimplifiedDomainEliding() ||
+      elide_after_web_contents_interaction_animation_) {
+    return;
+  }
+  GetRenderText()->SetElideBehavior(gfx::NO_ELIDE);
+  elide_after_web_contents_interaction_animation_ =
+      std::make_unique<ElideAnimation>(this, GetRenderText());
+  std::vector<gfx::Range> ranges_surrounding_simplified_domain;
+  gfx::Range simplified_domain =
+      GetSimplifiedDomainBounds(&ranges_surrounding_simplified_domain);
+  elide_after_web_contents_interaction_animation_->Start(
+      simplified_domain, 0 /* delay_ms */, ranges_surrounding_simplified_domain,
+      GetOmniboxColor(GetThemeProvider(),
+                      OmniboxPart::LOCATION_BAR_TEXT_DIMMED),
+      SK_ColorTRANSPARENT);
+}
+
 void OmniboxViewViews::ElideURL() {
   DCHECK(OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction() ||
          OmniboxFieldTrial::ShouldRevealPathQueryRefOnHover());

chrome/browser/ui/views/omnibox/omnibox_view_views.h

@@ -40,6 +40,7 @@ class OmniboxClient;
 class OmniboxPopupContentsView;
 
 namespace content {
+struct FocusedNodeDetails;
 class WebContents;
 }  // namespace content
 
@@ -161,6 +162,7 @@ class OmniboxViewViews : public OmniboxView,
   // content::WebContentsObserver:
   void DidFinishNavigation(content::NavigationHandle* navigation) override;
   void DidGetUserInteraction(const blink::WebInputEvent& event) override;
+  void OnFocusChangedInPage(content::FocusedNodeDetails* details) override;
 
   // For testing only.
   OmniboxPopupContentsView* GetPopupContentsViewForTesting() const {
@@ -186,6 +188,9 @@ class OmniboxViewViews : public OmniboxView,
   FRIEND_TEST_ALL_PREFIXES(
       OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
       MouseClick);
+  FRIEND_TEST_ALL_PREFIXES(
+      OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
+      FocusingEditableNode);
   FRIEND_TEST_ALL_PREFIXES(
       OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
       BoundsChanged);
@@ -512,6 +517,12 @@ class OmniboxViewViews : public OmniboxView,
   // display.
   void OnShouldPreventElisionChanged();
 
+  // Elides the URL to a simplified version of the domain with an animation.
+  // This should be called when a user interaction with the web contents
+  // triggers elision. Does nothing if the relevant field trial is disabled or
+  // the URL is not eligible for eliding.
+  void MaybeElideURLWithAnimationFromInteraction();
+
   // The methods below elide to or unelide from a simplified version of the URL.
   // Callers should ensure that the URL is valid before calling.
   //

chrome/browser/ui/views/omnibox/omnibox_view_views_unittest.cc

@@ -32,6 +32,7 @@
 #include "components/omnibox/browser/omnibox_edit_model.h"
 #include "components/omnibox/browser/test_location_bar_model.h"
 #include "components/omnibox/common/omnibox_features.h"
+#include "content/public/browser/focused_node_details.h"
 #include "content/public/test/browser_task_environment.h"
 #include "content/public/test/mock_navigation_handle.h"
 #include "content/public/test/test_renderer_host.h"
@@ -2006,7 +2007,8 @@ TEST_P(OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
                 0, kSimplifiedDomainDisplayUrlSubdomainAndScheme.size())));
 }
 
-// Tests that mouse clicks do not count as user interactions and elide the URL.
+// Tests that mouse clicks do not count as user interactions and do not elide
+// the URL.
 TEST_P(OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest, MouseClick) {
   SetUpSimplifiedDomainTest();
 
@@ -2027,6 +2029,37 @@ TEST_P(OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest, MouseClick) {
   EXPECT_FALSE(elide_animation);
 }
 
+// Tests that focusing an editable node does count as a user interaction and
+// elides the URL.
+TEST_P(OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
+       FocusingEditableNode) {
+  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())));
+
+  // Focusing a non-editable node should not run the fade-out animation.
+  content::FocusedNodeDetails details;
+  details.is_editable_node = false;
+  omnibox_view()->OnFocusChangedInPage(&details);
+  OmniboxViewViews::ElideAnimation* elide_animation =
+      omnibox_view()->GetElideAfterInteractionAnimationForTesting();
+  EXPECT_FALSE(elide_animation);
+
+  // Focusing an editable node should run the fade-out animation.
+  details.is_editable_node = true;
+  omnibox_view()->OnFocusChangedInPage(&details);
+  elide_animation =
+      omnibox_view()->GetElideAfterInteractionAnimationForTesting();
+  ASSERT_TRUE(elide_animation);
+  EXPECT_TRUE(elide_animation->IsAnimating());
+}
+
 // Tests that simplified domain elisions are re-applied when the omnibox's
 // bounds change.
 TEST_P(OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest, BoundsChanged) {