Fixing Omnibox action buttons

Fixing action buttons:
- Check if the location bar is in steady state.
  If yes, the Copy button is displayed.
  If not, if there is selected text the Copy button is displayed.
  In other cases, the Copy button is hidden.
- display Paste & Search for Copied Text buttons when omnibox is empty.
- hide Select & Select All buttons when omnibox is empty.
- display Select All button when selected text and hide Select button.

Corresponding integration tests dded.


Bug: 864984

Change-Id: I321546571272235eac0b749753f041bff0922608
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1786639
Commit-Queue: Ewann Pellé <ewannpv@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705068}

ios/chrome/browser/ui/location_bar/location_bar_view_controller.mm

@@ -482,7 +482,8 @@ const double kFullscreenProgressBadgeViewThreshold = 0.85;
 }
 
 - (BOOL)canPerformAction:(SEL)action withSender:(id)sender {
-  if (action == @selector(copy:)) {
+  // Allow copying if the steady location bar is visible.
+  if (!self.locationBarSteadyView.hidden && action == @selector(copy:)) {
     return YES;
   }
 

ios/chrome/browser/ui/omnibox/omnibox_egtest.mm

@@ -7,6 +7,7 @@
 
 #include "base/bind.h"
 #include "base/strings/sys_string_conversions.h"
+#import "base/test/ios/wait_util.h"
 #import "ios/chrome/browser/ui/content_suggestions/ntp_home_constant.h"
 #import "ios/chrome/browser/ui/omnibox/popup/omnibox_popup_row.h"
 #include "ios/chrome/grit/ios_strings.h"
@@ -24,6 +25,8 @@
 #error "This file requires ARC support."
 #endif
 
+using base::test::ios::kWaitForUIElementTimeout;
+
 namespace {
 
 // Web page 1.
@@ -60,6 +63,49 @@ std::unique_ptr<net::test_server::HttpResponse> StandardResponse(
   return nil;
 }
 
+// Returns visit Copied Link button matcher from UIMenuController.
+id<GREYMatcher> VisitCopiedLinkButton() {
+  NSString* a11yLabelCopiedLink =
+      l10n_util::GetNSString(IDS_IOS_VISIT_COPIED_LINK);
+  return grey_allOf(grey_accessibilityLabel(a11yLabelCopiedLink),
+                    chrome_test_util::SystemSelectionCallout(), nil);
+}
+
+// Returns Paste button matcher from UIMenuController.
+id<GREYMatcher> PasteButton() {
+  NSString* a11yLabelPaste = @"Paste";
+  return grey_allOf(grey_accessibilityLabel(a11yLabelPaste),
+                    chrome_test_util::SystemSelectionCallout(), nil);
+}
+
+// Returns Select button from UIMenuController.
+id<GREYMatcher> SelectButton() {
+  NSString* a11yLabelSelect = @"Select";
+  return grey_allOf(grey_accessibilityLabel(a11yLabelSelect),
+                    chrome_test_util::SystemSelectionCallout(), nil);
+}
+
+// Returns Select All button from UIMenuController.
+id<GREYMatcher> SelectAllButton() {
+  NSString* a11yLabelSelectAll = @"Select All";
+  return grey_allOf(grey_accessibilityLabel(a11yLabelSelectAll),
+                    chrome_test_util::SystemSelectionCallout(), nil);
+}
+
+// Returns Cut button from UIMenuController.
+id<GREYMatcher> CutButton() {
+  NSString* a11yLabelCut = @"Cut";
+  return grey_allOf(grey_accessibilityLabel(a11yLabelCut),
+                    chrome_test_util::SystemSelectionCallout(), nil);
+}
+
+// Returns Search Copied Text button from UIMenuController.
+id<GREYMatcher> SearchCopiedTextButton() {
+  NSString* a11yLabelsearchCopiedText = @"Search for Copied Text";
+  return grey_allOf(grey_accessibilityLabel(a11yLabelsearchCopiedText),
+                    chrome_test_util::SystemSelectionCallout(), nil);
+}
+
 }  //  namespace
 
 #pragma mark - Steady state tests
@@ -120,6 +166,15 @@ std::unique_ptr<net::test_server::HttpResponse> StandardResponse(
                                           SystemSelectionCalloutCopyButton()]
       assertWithMatcher:grey_notNil()];
 
+  // Pressing should not allow pasting when pasteboard is empty.
+  // Verify that system text selection callout is not displayed.
+  [[EarlGrey selectElementWithMatcher:VisitCopiedLinkButton()]
+      assertWithMatcher:grey_nil()];
+  [[EarlGrey selectElementWithMatcher:SearchCopiedTextButton()]
+      assertWithMatcher:grey_nil()];
+  [[EarlGrey selectElementWithMatcher:PasteButton()]
+      assertWithMatcher:grey_nil()];
+
   [self checkLocationBarSteadyState];
 
   // Tapping it should copy the URL.
@@ -136,27 +191,22 @@ std::unique_ptr<net::test_server::HttpResponse> StandardResponse(
                         hasSuffix:base::SysUTF8ToNSString(kPage1URL)];
                   }];
   // Wait for copy to happen or timeout after 5 seconds.
-  BOOL success = [copyCondition waitWithTimeout:5];
-  GREYAssertTrue(success, @"Copying page 1 URL failed");
+  GREYAssertTrue([copyCondition waitWithTimeout:5],
+                 @"Copying page 1 URL failed");
 
   // Go to another web page.
   [self openPage2];
 
   // Visit copied link should now be available.
-  NSString* a11yLabelPasteGo =
-      l10n_util::GetNSString(IDS_IOS_VISIT_COPIED_LINK);
-  id<GREYMatcher> pasteAndGoMatcher =
-      grey_allOf(grey_accessibilityLabel(a11yLabelPasteGo),
-                 chrome_test_util::SystemSelectionCallout(), nil);
   [[EarlGrey selectElementWithMatcher:chrome_test_util::DefocusedLocationView()]
       performAction:grey_longPress()];
-  [[EarlGrey selectElementWithMatcher:pasteAndGoMatcher]
+  [[EarlGrey selectElementWithMatcher:VisitCopiedLinkButton()]
       assertWithMatcher:grey_notNil()];
 
   [self checkLocationBarSteadyState];
 
   // Tapping it should navigate to Page 1.
-  [[EarlGrey selectElementWithMatcher:pasteAndGoMatcher]
+  [[EarlGrey selectElementWithMatcher:VisitCopiedLinkButton()]
       performAction:grey_tap()];
 
   [ChromeEarlGrey waitForPageToFinishLoading];
@@ -202,8 +252,8 @@ std::unique_ptr<net::test_server::HttpResponse> StandardResponse(
                         hasSuffix:base::SysUTF8ToNSString(kPage1URL)];
                   }];
   // Wait for copy to happen or timeout after 5 seconds.
-  BOOL success = [copyCondition waitWithTimeout:5];
-  GREYAssertTrue(success, @"Copying page 1 URL failed");
+  GREYAssertTrue([copyCondition waitWithTimeout:5],
+                 @"Copying page 1 URL failed");
 
   // Defocus the omnibox.
   if ([ChromeEarlGrey isIPadIdiom]) {
@@ -268,3 +318,182 @@ std::unique_ptr<net::test_server::HttpResponse> StandardResponse(
 }
 
 @end
+
+#pragma mark - Edit state tests
+
+@interface LocationBarEditStateTestCase : ChromeTestCase
+@end
+
+@implementation LocationBarEditStateTestCase
+
+- (void)setUp {
+  [super setUp];
+
+  [ChromeEarlGrey clearBrowsingHistory];
+
+  // Clear the pasteboard in case there is a URL copied.
+  UIPasteboard* pasteboard = UIPasteboard.generalPasteboard;
+  [pasteboard setValue:@"" forPasteboardType:UIPasteboardNameGeneral];
+}
+
+// Copy button should be hidden when the omnibox is empty otherwise it should be
+// displayed. Paste button should be hidden when pasteboard is empty otherwise
+// it should be displayed. Select & SelectAll buttons should be hidden when the
+// omnibox is empty.
+- (void)testEmptyOmnibox {
+  // Focus omnibox.
+  [self focusFakebox];
+  [[EarlGrey selectElementWithMatcher:chrome_test_util::Omnibox()]
+      assertWithMatcher:grey_sufficientlyVisible()];
+  [[EarlGrey selectElementWithMatcher:chrome_test_util::Omnibox()]
+      performAction:grey_tap()];
+
+  // Pressing should not allow copying when omnibox is empty.
+  // Wait for Copy button to appear or timeout after 2 seconds.
+  GREYCondition* CopyButtonIsDisplayed = [GREYCondition
+      conditionWithName:@"Copy button display condition"
+                  block:^BOOL {
+                    NSError* error = nil;
+                    [[EarlGrey selectElementWithMatcher:
+                                   chrome_test_util::
+                                       SystemSelectionCalloutCopyButton()]
+                        assertWithMatcher:grey_notNil()
+                                    error:&error];
+                    return error == nil;
+                  }];
+  // Verify that system text selection callout is not displayed.
+  GREYAssertFalse(
+      [CopyButtonIsDisplayed waitWithTimeout:kWaitForUIElementTimeout],
+      @"Copy button should not be displayed");
+
+  // Pressing should not allow select or selectAll when omnibox is empty.
+  // Verify that system text selection callout is not displayed.
+  [[EarlGrey selectElementWithMatcher:SelectButton()]
+      assertWithMatcher:grey_nil()];
+  [[EarlGrey selectElementWithMatcher:SelectAllButton()]
+      assertWithMatcher:grey_nil()];
+
+  // Pressing should not allow pasting when pasteboard is empty.
+  // Verify that system text selection callout is not displayed.
+  [[EarlGrey selectElementWithMatcher:VisitCopiedLinkButton()]
+      assertWithMatcher:grey_nil()];
+  [[EarlGrey selectElementWithMatcher:SearchCopiedTextButton()]
+      assertWithMatcher:grey_nil()];
+  [[EarlGrey selectElementWithMatcher:PasteButton()]
+      assertWithMatcher:grey_nil()];
+
+  // Writing in the omnibox field.
+  [[EarlGrey selectElementWithMatcher:chrome_test_util::Omnibox()]
+      performAction:grey_typeText(@"this is a test")];
+
+  // Click on the omnibox.
+  [[EarlGrey selectElementWithMatcher:chrome_test_util::Omnibox()]
+      performAction:grey_tap()];
+
+  // SelectAll the omnibox field.
+  GREYCondition* SelectAllButtonIsDisplayed = [GREYCondition
+      conditionWithName:@"SelectAll button display condition"
+                  block:^BOOL {
+                    NSError* error = nil;
+                    [[EarlGrey selectElementWithMatcher:SelectAllButton()]
+                        performAction:grey_tap()
+                                error:&error];
+                    return error == nil;
+                  }];
+  GREYAssertTrue(
+      [SelectAllButtonIsDisplayed waitWithTimeout:kWaitForUIElementTimeout],
+      @"SelectAll button display failed");
+
+  // Cut the text.
+  [[EarlGrey selectElementWithMatcher:CutButton()] performAction:grey_tap()];
+
+  // Pressing should allow pasting.
+  // Click on the omnibox.
+  [[EarlGrey selectElementWithMatcher:chrome_test_util::Omnibox()]
+      performAction:grey_tap()];
+  // Verify that system text selection callout is displayed (Search Copied
+  // Text).
+  GREYCondition* searchCopiedTextButtonIsDisplayed = [GREYCondition
+      conditionWithName:@"Search Copied Text button display condition"
+                  block:^BOOL {
+                    NSError* error = nil;
+                    [[EarlGrey
+                        selectElementWithMatcher:SearchCopiedTextButton()]
+                        assertWithMatcher:grey_notNil()
+                                    error:&error];
+                    return error == nil;
+                  }];
+  GREYAssertTrue([searchCopiedTextButtonIsDisplayed
+                     waitWithTimeout:kWaitForUIElementTimeout],
+                 @"Search Copied Text button display failed");
+  // Verify that system text selection callout is displayed (Paste).
+  [[EarlGrey selectElementWithMatcher:PasteButton()]
+      assertWithMatcher:grey_notNil()];
+}
+
+// Select & SelectAll buttons should be displayed when the omnibox is not empty
+// and no text is selected. If the selected text is a sub part of the omnibox
+// fied, Select button should be hidden & SelectAll button should be displayed.
+// If the selected text is the entire omnibox field, select & SelectAll button
+// should be hidden.
+- (void)testSelection {
+  // Focus omnibox.
+  [self focusFakebox];
+  [[EarlGrey selectElementWithMatcher:chrome_test_util::Omnibox()]
+      assertWithMatcher:grey_sufficientlyVisible()];
+  // Write in the omnibox field.
+  [[EarlGrey selectElementWithMatcher:chrome_test_util::Omnibox()]
+      performAction:grey_typeText(@"this is a test")];
+
+  [[EarlGrey selectElementWithMatcher:chrome_test_util::Omnibox()]
+      performAction:grey_tap()];
+
+  // Pressing should allow select and selectAll.
+  // Wait for UIMenuController to appear or timeout after 2 seconds.
+  GREYCondition* SelectButtonIsDisplayed = [GREYCondition
+      conditionWithName:@"Select button display condition"
+                  block:^BOOL {
+                    NSError* error = nil;
+                    [[EarlGrey selectElementWithMatcher:SelectButton()]
+                        assertWithMatcher:grey_notNil()
+                                    error:&error];
+                    return error == nil;
+                  }];
+  // Verify that system text selection callout is displayed.
+  GREYAssertTrue(
+      [SelectButtonIsDisplayed waitWithTimeout:kWaitForUIElementTimeout],
+      @"Select button display failed");
+  [[EarlGrey selectElementWithMatcher:SelectAllButton()]
+      assertWithMatcher:grey_notNil()];
+
+  // Pressing select should allow copy.
+  // select should be hidden.
+  [[EarlGrey selectElementWithMatcher:SelectButton()] performAction:grey_tap()];
+  [[EarlGrey selectElementWithMatcher:chrome_test_util::
+                                          SystemSelectionCalloutCopyButton()]
+      assertWithMatcher:grey_notNil()];
+  [[EarlGrey selectElementWithMatcher:SelectButton()]
+      assertWithMatcher:grey_nil()];
+
+  // Pressing selectAll should allow copy.
+  // selectAll should be hidden.
+  [[EarlGrey selectElementWithMatcher:SelectAllButton()]
+      performAction:grey_tap()];
+  [[EarlGrey selectElementWithMatcher:chrome_test_util::
+                                          SystemSelectionCalloutCopyButton()]
+      assertWithMatcher:grey_notNil()];
+  [[EarlGrey selectElementWithMatcher:SelectAllButton()]
+      assertWithMatcher:grey_nil()];
+}
+
+#pragma mark - Helpers
+
+// Taps the fake omnibox and waits for the real omnibox to be visible.
+- (void)focusFakebox {
+  [[EarlGrey selectElementWithMatcher:chrome_test_util::FakeOmnibox()]
+      performAction:grey_tap()];
+  [ChromeEarlGrey
+      waitForSufficientlyVisibleElementWithMatcher:chrome_test_util::Omnibox()];
+}
+
+@end

ios/chrome/browser/ui/omnibox/omnibox_text_field_ios.mm

@@ -616,15 +616,22 @@ NSString* const kOmniboxFadeAnimationKey = @"OmniboxFadeAnimation";
 }
 
 - (BOOL)canPerformAction:(SEL)action withSender:(id)sender {
-  // If there is selected text, show copy and cut.
+  // If the text is not empty and there is selected text, show copy and cut.
   if ([self textInRange:self.selectedTextRange].length > 0 &&
       (action == @selector(cut:) || action == @selector(copy:))) {
     return YES;
   }
 
-  // If there is no selected text, show select and selectAll.
-  if ([self textInRange:self.selectedTextRange].length == 0 &&
-      (action == @selector(select:) || action == @selector(selectAll:))) {
+  // If the text is not empty and there is no selected text, show select
+  if (self.text.length > 0 &&
+      [self textInRange:self.selectedTextRange].length == 0 &&
+      action == @selector(select:)) {
+    return YES;
+  }
+
+  // If selected text is les than the text length, show selectAll.
+  if ([self textInRange:self.selectedTextRange].length != self.text.length &&
+      action == @selector(selectAll:)) {
     return YES;
   }