Local NTP: show UI (X button) for removing suggestions from realbox

R=mahmadi@chromium.org

Bug: 1002689
Change-Id: I83eea8ade06f0d7f01d9e04c6b091bf0185bfc87
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1831813
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Auto-Submit: Dan Beam <dbeam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702086}

chrome/browser/resources/local_ntp/externs.js

@@ -409,6 +409,7 @@ let ACMatchClassification;
  *   inlineAutocompletion: string,
  *   isSearchType: boolean,
  *   fillIntoEdit: string,
+ *   supportsDeletion: boolean,
  *   swapContentsAndDescription: boolean,
  *   type: string,
  * }}
@@ -416,7 +417,7 @@ let ACMatchClassification;
 let AutocompleteMatch;
 
 /** @enum {number} */
-let AutocompleteResultStatus;
+const AutocompleteResultStatus = {};
 
 /**
  * @typedef {{
@@ -488,6 +489,7 @@ configData.translatedStrings.otherError;
 configData.translatedStrings.permissionError;
 configData.translatedStrings.ready;
 configData.translatedStrings.realboxSeparator;
+configData.translatedStrings.removeSuggestion;
 configData.translatedStrings.removeThumbnailTooltip;
 configData.translatedStrings.restoreDefaultBackground;
 configData.translatedStrings.restoreDefaultLinks;

chrome/browser/resources/local_ntp/local_ntp.css

@@ -229,6 +229,10 @@ body.hide-fakebox #fakebox {
   white-space: nowrap;
 }
 
+#realbox-matches a.removable {
+  padding-inline-end: 48px;
+}
+
 #realbox-matches .search-icon {
   -webkit-mask-size: 16px;
 }
@@ -277,6 +281,40 @@ html[dir=rtl] #realbox-matches :-webkit-any(.clock-icon, .url-icon) {
   color: rgb(var(--GB600-rgb));
 }
 
+#realbox-matches .remove-match {
+  border-radius: 50%;
+  height: 24px;
+  position: absolute;
+  right: 16px;
+  top: 4px;
+  width: 24px;
+}
+
+[dir=rtl] #realbox-matches .remove-match {
+  left: 16px;
+  right: auto;
+}
+
+#realbox-matches .remove-match:hover {
+  background: rgba(var(--GG900-rgb), .08);
+}
+
+#realbox-matches .remove-match:focus-within {
+  background: rgba(var(--GG900-rgb), .16);
+}
+
+#realbox-matches .remove-icon {
+  -webkit-appearance: none;
+  -webkit-mask-image: url(../../../../ui/webui/resources/images/icon_clear.svg);
+  -webkit-mask-position: center;
+  -webkit-mask-repeat: no-repeat;
+  -webkit-mask-size: 16px;
+  background-color: rgb(var(--GG900-rgb));
+  border: none;
+  height: 100%;
+  width: 100%;
+}
+
 #fakebox > input {
   bottom: 0;
   box-sizing: border-box;

chrome/browser/resources/local_ntp/local_ntp.js

@@ -92,6 +92,9 @@ const CLASSES = {
   LEFT_ALIGN_ATTRIBUTION: 'left-align-attribution',
   // Vertically centers the most visited section for a non-Google provided page.
   NON_GOOGLE_PAGE: 'non-google-page',
+  REMOVABLE: 'removable',
+  REMOVE_ICON: 'remove-icon',
+  REMOVE_MATCH: 'remove-match',
   SEARCH_ICON: 'search-icon',  // Magnifying glass/search icon.
   SELECTED: 'selected',  // A selected (via up/down arrow key) realbox match.
   SHOW_ELEMENT: 'show-element',
@@ -1220,8 +1223,10 @@ function onRealboxKeyDown(e) {
   });
 
   if (key === 'Delete' && e.shiftKey && !e.altKey && !e.ctrlKey && !e.metaKey) {
-    window.chrome.embeddedSearch.searchBox.deleteAutocompleteMatch(selected);
-    e.preventDefault();
+    if (autocompleteMatches[selected].supportsDeletion) {
+      window.chrome.embeddedSearch.searchBox.deleteAutocompleteMatch(selected);
+      e.preventDefault();
+    }
     return;
   }
 
@@ -1284,8 +1289,15 @@ function onRealboxInput() {
 function onRealboxWrapperFocusIn(e) {
   if (e.target.matches(`#${IDS.REALBOX}`) && !$(IDS.REALBOX).value) {
     window.chrome.embeddedSearch.searchBox.queryAutocomplete('');
-  } else if (e.target.matches(`#${IDS.REALBOX_MATCHES} a`)) {
-    const selectedIndex = selectMatchEl(e.target);
+  } else if (e.target.matches(`#${IDS.REALBOX_MATCHES} *`)) {
+    let target = e.target;
+    while (target && target.nodeName !== 'A') {
+      target = target.parentNode;
+    }
+    if (!target) {
+      return;
+    }
+    const selectedIndex = selectMatchEl(target);
     // It doesn't really make sense to use fillFromMatch() here as the focus
     // change drops the selection (and is probably just noisy to
     // screenreaders).
@@ -1374,10 +1386,13 @@ function overrideExecutableTimeoutForTesting(timeout) {
  */
 function populateAutocompleteMatches(matches) {
   const realboxMatchesEl = document.createElement('div');
+  realboxMatchesEl.role = 'listbox';
 
-  for (const [i, match] of matches.entries()) {
+  for (let i = 0; i < matches.length; ++i) {
+    const match = matches[i];
     const matchEl = document.createElement('a');
     matchEl.href = match.destinationUrl;
+    matchEl.role = 'option';
 
     let iconClass;
     if (match.isSearchType) {
@@ -1411,6 +1426,26 @@ function populateAutocompleteMatches(matches) {
       col.forEach(colEl => matchEl.appendChild(colEl));
     }
 
+    if (match.supportsDeletion) {
+      const icon = document.createElement('button');
+      icon.title = configData.translatedStrings.removeSuggestion;
+      icon.classList.add(CLASSES.REMOVE_ICON);
+      icon.onclick = e => {
+        window.chrome.embeddedSearch.searchBox.deleteAutocompleteMatch(i);
+        e.preventDefault();
+      };
+
+      const remove = document.createElement('div');
+      remove.classList.add(CLASSES.REMOVE_MATCH);
+
+      remove.appendChild(icon);
+      matchEl.appendChild(remove);
+      matchEl.classList.add(CLASSES.REMOVABLE);
+    }
+
+    // TODO(crbug.com/1002689): set a more useful aria-label on |matchEl|, as
+    // "Remove suggestion" is now uttered when navigating through matches.
+
     realboxMatchesEl.append(matchEl);
   }
 

chrome/browser/search/local_ntp_source.cc

@@ -298,6 +298,8 @@ std::unique_ptr<base::DictionaryValue> GetTranslatedStrings(bool is_google) {
     // Realbox
     AddString(translated_strings.get(), "realboxSeparator",
               IDS_AUTOCOMPLETE_MATCH_DESCRIPTION_SEPARATOR);
+    AddString(translated_strings.get(), "removeSuggestion",
+              IDS_OMNIBOX_REMOVE_SUGGESTION);
 
     // Promos
     AddString(translated_strings.get(), "dismissPromo", IDS_NTP_DISMISS_PROMO);

chrome/browser/ui/search/search_tab_helper.cc

@@ -88,6 +88,7 @@ std::vector<chrome::mojom::AutocompleteMatchPtr> CreateAutocompleteMatches(
     mojom_match->swap_contents_and_description =
         match.swap_contents_and_description;
     mojom_match->type = AutocompleteMatchType::ToString(match.type);
+    mojom_match->supports_deletion = match.SupportsDeletion();
     matches.push_back(std::move(mojom_match));
   }
   return matches;

chrome/common/search.mojom

@@ -50,6 +50,7 @@ struct AutocompleteMatch {
   bool is_search_type;  // Result of AutocompleteMatch::IsSearchType().
   string type;  // Result of AutocompleteMatchType::ToString().
   bool swap_contents_and_description;
+  bool supports_deletion;
 };
 
 enum AutocompleteResultStatus {

chrome/renderer/searchbox/searchbox_extension.cc

@@ -432,6 +432,7 @@ base::Value CreateAutocompleteMatches(
     dict.SetBoolKey("swapContentsAndDescription",
                     match->swap_contents_and_description);
     dict.SetStringKey("type", match->type);
+    dict.SetBoolKey("supportsDeletion", match->supports_deletion);
     list.Append(std::move(dict));
   }
   return list;

chrome/test/data/local_ntp/realbox_browsertest.js

@@ -19,6 +19,7 @@ test.realbox.IDS = {
  * @const
  */
 test.realbox.CLASSES = {
+  REMOVE_ICON: 'remove-icon',
   SELECTED: 'selected',
   SHOW_MATCHES: 'show-matches',
 };
@@ -495,11 +496,11 @@ test.realbox.testDeleteAutocompleteResultShiftDeleteWithNoMatches = function() {
   assertFalse(keyEvent.defaultPrevented);
 };
 
-test.realbox.testDeleteAutocompleteResultShiftDeleteCantRemove = function() {
+test.realbox.testUnsupportedDeletion = function() {
   test.realbox.realboxEl.value = 'hello world';
   test.realbox.realboxEl.dispatchEvent(new CustomEvent('input'));
 
-  const matches = [test.realbox.getSearchMatch(), test.realbox.getUrlMatch()];
+  const matches = [test.realbox.getSearchMatch({supportsDeletion: false})];
   chrome.embeddedSearch.searchBox.onqueryautocompletedone(
       {input: test.realbox.realboxEl.value, matches});
 
@@ -510,24 +511,25 @@ test.realbox.testDeleteAutocompleteResultShiftDeleteCantRemove = function() {
     shiftKey: true,
   });
   test.realbox.realboxEl.dispatchEvent(keyEvent);
-  assertTrue(keyEvent.defaultPrevented);
-
-  assertEquals(1, test.realbox.deletedLines.length);
-  assertEquals(0, test.realbox.deletedLines[0]);
-
-  assertEquals(2, $(test.realbox.IDS.REALBOX_MATCHES).children.length);
+  assertFalse(keyEvent.defaultPrevented);
 
+  // The below 2 statements shouldn't really happen in updated code but isn't
+  // terrible idea to keep testing for now. This is because SupportsDeletion()
+  // is now propagated to the client, so we shouldn't allow users (via the UI)
+  // to attempt to delete non-deletable things.
   chrome.embeddedSearch.searchBox.ondeleteautocompletematch(
       {success: false, matches: []});
-
-  assertEquals(2, $(test.realbox.IDS.REALBOX_MATCHES).children.length);
+  assertEquals(1, $(test.realbox.IDS.REALBOX_MATCHES).children.length);
 };
 
-test.realbox.testDeleteAutocompleteResultShiftDeleteCanRemove = function() {
+test.realbox.testSupportedDeletion = function() {
   test.realbox.realboxEl.value = 'hello world';
   test.realbox.realboxEl.dispatchEvent(new CustomEvent('input'));
 
-  const matches = [test.realbox.getSearchMatch(), test.realbox.getUrlMatch()];
+  const matches = [
+    test.realbox.getSearchMatch({supportsDeletion: true}),
+    test.realbox.getUrlMatch({supportsDeletion: true}),
+  ];
   chrome.embeddedSearch.searchBox.onqueryautocompletedone(
       {input: test.realbox.realboxEl.value, matches});
 
@@ -560,3 +562,24 @@ test.realbox.testDeleteAutocompleteResultShiftDeleteCanRemove = function() {
 
   assertEquals(1, $(test.realbox.IDS.REALBOX_MATCHES).children.length);
 };
+
+test.realbox.testRemoveIcon = function() {
+  test.realbox.realboxEl.value = 'hello world';
+  test.realbox.realboxEl.dispatchEvent(new CustomEvent('input'));
+
+  const matches = [test.realbox.getSearchMatch({supportsDeletion: true})];
+  chrome.embeddedSearch.searchBox.onqueryautocompletedone(
+      {input: test.realbox.realboxEl.value, matches});
+
+  const sel = `#${test.realbox.IDS.REALBOX_MATCHES}
+               .${test.realbox.CLASSES.REMOVE_ICON}`;
+  document.querySelector(sel).click();
+
+  assertEquals(1, test.realbox.deletedLines.length);
+  assertEquals(0, test.realbox.deletedLines[0]);
+
+  chrome.embeddedSearch.searchBox.ondeleteautocompletematch(
+      {success: true, matches: []});
+
+  assertEquals(0, $(test.realbox.IDS.REALBOX_MATCHES).children.length);
+};