[settings] Present a confirmation dialog before deleting search engines.

Screenshot: https://imgur.com/RSpKUbq

Bug: 1263679
Change-Id: I79f925608625b8c534dd8f00ed8aabb4f097052a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3400641
Reviewed-by: Angela Yoeurng <yoangela@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/main@{#961683}

chrome/app/settings_strings.grdp

@@ -1933,6 +1933,12 @@
   <message name="IDS_SETTINGS_SEARCH_ENGINES_EDIT_SEARCH_ENGINE" desc="Title for a dialog that allows editing an existing search engine.">
     Edit search engine
   </message>
+  <message name="IDS_SETTINGS_SEARCH_ENGINES_DELETE_CONFIRMATION_TITLE" desc="Title for a dialog that confirms that a user wants to delete an existing search engine.">
+    Delete search engine
+  </message>
+  <message name="IDS_SETTINGS_SEARCH_ENGINES_DELETE_CONFIRMATION_DESCRIPTION" desc="Description text for a dialog that confirms that a user wants to delete an existing search engine.">
+    Are you sure you want to delete this search engine?
+  </message>
   <message name="IDS_SETTINGS_SEARCH_ENGINES_DEFAULT_ENGINES" desc="Label for 'default' Search Engines section">
     Default search engines
   </message>

chrome/app/settings_strings_grdp/IDS_SETTINGS_SEARCH_ENGINES_DELETE_CONFIRMATION_DESCRIPTION.png.sha1

@@ -0,0 +1 @@
+0b617a1888fbe8dff256d2f67a2a85188131e319

chrome/app/settings_strings_grdp/IDS_SETTINGS_SEARCH_ENGINES_DELETE_CONFIRMATION_TITLE.png.sha1

@@ -0,0 +1 @@
+0b617a1888fbe8dff256d2f67a2a85188131e319

chrome/browser/resources/settings/lazy_load.ts

@@ -149,7 +149,8 @@ export {SafeBrowsingSetting, SettingsSecurityPageElement} from './privacy_page/s
 export {SettingsResetPageElement} from './reset_page/reset_page.js';
 export {SettingsResetProfileDialogElement} from './reset_page/reset_profile_dialog.js';
 export {SettingsOmniboxExtensionEntryElement} from './search_engines_page/omnibox_extension_entry.js';
-export {SettingsSearchEngineDialogElement} from './search_engines_page/search_engine_dialog.js';
+export {SettingsSearchEngineDeleteConfirmationDialogElement} from './search_engines_page/search_engine_delete_confirmation_dialog.js';
+export {SettingsSearchEngineEditDialogElement} from './search_engines_page/search_engine_edit_dialog.js';
 export {SettingsSearchEngineEntryElement} from './search_engines_page/search_engine_entry.js';
 export {SettingsSearchEnginesListElement} from './search_engines_page/search_engines_list.js';
 export {SettingsSearchEnginesPageElement} from './search_engines_page/search_engines_page.js';

chrome/browser/resources/settings/search_engines_page/search_engine_delete_confirmation_dialog.html

@@ -0,0 +1,14 @@
+    <style include="settings-shared">
+    </style>
+    <cr-dialog id="dialog" close-text="$i18n{close}">
+      <div slot="title">$i18n{searchEnginesDeleteConfirmationTitle}</div>
+      <div slot="body">$i18n{searchEnginesDeleteConfirmationDescription}</div>
+      <div slot="button-container">
+        <cr-button class="cancel-button" on-click="cancel_" id="cancelButton">
+          $i18n{cancel}
+        </cr-button>
+        <cr-button class="delete-button" on-click="delete_" id="deleteButton">
+          $i18n{delete}
+        </cr-button>
+      </div>
+    </cr-dialog>

chrome/browser/resources/settings/search_engines_page/search_engine_delete_confirmation_dialog.ts

@@ -0,0 +1,84 @@
+// Copyright 2022 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+/**
+ * @fileoverview 'settings-search-engine-delete-confirmation-dialog' is a
+ * component for confirming that the user wants to delete a search engine.
+ */
+import 'chrome://resources/cr_elements/cr_button/cr_button.m.js';
+import 'chrome://resources/cr_elements/cr_dialog/cr_dialog.m.js';
+
+import {CrButtonElement} from 'chrome://resources/cr_elements/cr_button/cr_button.m.js';
+import {CrDialogElement} from 'chrome://resources/cr_elements/cr_dialog/cr_dialog.m.js';
+import {WebUIListenerMixin} from 'chrome://resources/js/web_ui_listener_mixin.js';
+import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
+
+import {loadTimeData} from '../i18n_setup.js';
+
+import {SearchEngine, SearchEnginesBrowserProxy, SearchEnginesBrowserProxyImpl, SearchEnginesInfo} from './search_engines_browser_proxy.js';
+
+export interface SettingsSearchEngineDeleteConfirmationDialogElement {
+  $: {
+    dialog: CrDialogElement,
+    cancelButton: CrButtonElement,
+    deleteButton: CrButtonElement,
+  };
+}
+
+const SettingsSearchEngineDeleteConfirmationDialogElementBase =
+    WebUIListenerMixin(PolymerElement);
+
+export class SettingsSearchEngineDeleteConfirmationDialogElement extends
+    SettingsSearchEngineDeleteConfirmationDialogElementBase {
+  static get is() {
+    return 'settings-search-engine-delete-confirmation-dialog';
+  }
+
+  static get template() {
+    return html`{__html_template__}`;
+  }
+
+  static get properties() {
+    return {
+      model: Object,
+
+      isActiveSearchEnginesFlagEnabled_: {
+        type: Boolean,
+        value: () =>
+            loadTimeData.getBoolean('isActiveSearchEnginesFlagEnabled'),
+      },
+    };
+  }
+
+  model: SearchEngine|null;
+  private browserProxy_: SearchEnginesBrowserProxy =
+      SearchEnginesBrowserProxyImpl.getInstance();
+
+  connectedCallback() {
+    super.connectedCallback();
+    this.$.dialog.showModal();
+  }
+
+  private cancel_() {
+    this.$.dialog.cancel();
+  }
+
+  private delete_() {
+    if (this.model) {
+      this.browserProxy_.removeSearchEngine(this.model.modelIndex);
+    }
+    this.$.dialog.close();
+  }
+}
+
+declare global {
+  interface HTMLElementTagNameMap {
+    'settings-search-engine-delete-confirmation-dialog':
+        SettingsSearchEngineDeleteConfirmationDialogElement;
+  }
+}
+
+customElements.define(
+    SettingsSearchEngineDeleteConfirmationDialogElement.is,
+    SettingsSearchEngineDeleteConfirmationDialogElement);

chrome/browser/resources/settings/search_engines_page/search_engine_dialog.ts → chrome/browser/resources/settings/search_engines_page/search_engine_edit_dialog.ts

@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 
 /**
- * @fileoverview 'settings-search-engine-dialog' is a component for adding
+ * @fileoverview 'settings-search-engine-edit-dialog' is a component for adding
  * or editing a search engine entry.
  */
 import 'chrome://resources/cr_elements/cr_button/cr_button.m.js';
@@ -20,7 +20,7 @@ import {loadTimeData} from '../i18n_setup.js';
 
 import {SearchEngine, SearchEnginesBrowserProxy, SearchEnginesBrowserProxyImpl, SearchEnginesInfo} from './search_engines_browser_proxy.js';
 
-export interface SettingsSearchEngineDialogElement {
+export interface SettingsSearchEngineEditDialogElement {
   $: {
     actionButton: CrButtonElement,
     cancel: CrButtonElement,
@@ -31,13 +31,13 @@ export interface SettingsSearchEngineDialogElement {
   };
 }
 
-const SettingsSearchEngineDialogElementBase =
+const SettingsSearchEngineEditDialogElementBase =
     WebUIListenerMixin(PolymerElement);
 
-export class SettingsSearchEngineDialogElement extends
-    SettingsSearchEngineDialogElementBase {
+export class SettingsSearchEngineEditDialogElement extends
+    SettingsSearchEngineEditDialogElementBase {
   static get is() {
-    return 'settings-search-engine-dialog';
+    return 'settings-search-engine-edit-dialog';
   }
 
   static get template() {
@@ -189,9 +189,10 @@ export class SettingsSearchEngineDialogElement extends
 
 declare global {
   interface HTMLElementTagNameMap {
-    'settings-search-engine-dialog': SettingsSearchEngineDialogElement;
+    'settings-search-engine-edit-dialog': SettingsSearchEngineEditDialogElement;
   }
 }
 
 customElements.define(
-    SettingsSearchEngineDialogElement.is, SettingsSearchEngineDialogElement);
+    SettingsSearchEngineEditDialogElement.is,
+    SettingsSearchEngineEditDialogElement);

chrome/browser/resources/settings/search_engines_page/search_engine_entry.ts

@@ -82,9 +82,24 @@ export class SettingsSearchEngineEntryElement extends PolymerElement {
     return this.isActiveSearchEnginesFlagEnabled && this.engine.default;
   }
 
-  private onDeleteTap_() {
-    this.browserProxy_.removeSearchEngine(this.engine.modelIndex);
+  private onDeleteTap_(e: Event) {
+    e.preventDefault();
     this.closePopupMenu_();
+
+    if (!this.engine.shouldConfirmDeletion) {
+      this.browserProxy_.removeSearchEngine(this.engine.modelIndex);
+      return;
+    }
+
+    this.dispatchEvent(new CustomEvent('delete-search-engine', {
+      bubbles: true,
+      composed: true,
+      detail: {
+        engine: this.engine,
+        anchorElement: assert(
+            this.shadowRoot!.querySelector('cr-icon-button.icon-more-vert')!),
+      },
+    }));
   }
 
   private onDotsTap_() {

chrome/browser/resources/settings/search_engines_page/search_engines_browser_proxy.ts

@@ -29,6 +29,7 @@ export type SearchEngine = {
   keyword: string,
   modelIndex: number,
   name: string,
+  shouldConfirmDeletion: boolean,
   url: string,
   urlLocked: boolean,
 };

chrome/browser/resources/settings/search_engines_page/search_engines_page.html

@@ -72,10 +72,16 @@ <h2>$i18n{searchEnginesSearchEngines}</h2>
       $i18n{searchNoResults}
     </div>
 
-    <template is="dom-if" if="[[showDialog_]]" restamp>
-      <settings-search-engine-dialog model="[[dialogModel_]]"
-          on-close="onCloseDialog_">
-      </settings-search-engine-dialog>
+    <template is="dom-if" if="[[showEditDialog_]]" restamp>
+      <settings-search-engine-edit-dialog model="[[dialogModel_]]"
+          on-close="onCloseEditDialog_">
+      </settings-search-engine-edit-dialog>
+    </template>
+
+    <template is="dom-if" if="[[showDeleteConfirmationDialog_]]" restamp>
+      <settings-search-engine-delete-confirmation-dialog
+          model="[[dialogModel_]]" on-close="onCloseDeleteConfirmationDialog_">
+      </settings-search-engine-delete-confirmation-dialog>
     </template>
 
     <template is="dom-if" if="[[isActiveSearchEnginesFlagEnabled_]]">

chrome/browser/resources/settings/search_engines_page/search_engines_page.ts

@@ -13,7 +13,8 @@ import 'chrome://resources/js/cr.m.js';
 import 'chrome://resources/polymer/v3_0/iron-flex-layout/iron-flex-layout-classes.js';
 import '../controls/controlled_radio_button.js';
 import '../controls/settings_radio_group.js';
-import './search_engine_dialog.js';
+import './search_engine_delete_confirmation_dialog.js';
+import './search_engine_edit_dialog.js';
 import './search_engines_list.js';
 import './omnibox_extension_entry.js';
 import '../settings_shared_css.js';
@@ -36,6 +37,11 @@ type SearchEngineEditEvent = CustomEvent<{
   anchorElement: HTMLElement,
 }>;
 
+type SearchEngineDeleteEvent = CustomEvent<{
+  engine: SearchEngine,
+  anchorElement: HTMLElement,
+}>;
+
 export interface SettingsSearchEnginesPageElement {
   $: {
     addSearchEngine: HTMLElement,
@@ -125,7 +131,12 @@ export class SettingsSearchEnginesPageElement extends
         value: null,
       },
 
-      showDialog_: {
+      showEditDialog_: {
+        type: Boolean,
+        value: false,
+      },
+
+      showDeleteConfirmationDialog_: {
         type: Boolean,
         value: false,
       },
@@ -161,7 +172,8 @@ export class SettingsSearchEnginesPageElement extends
   private omniboxExtensionListBlurred_: boolean;
   private dialogModel_: SearchEngine|null;
   private dialogAnchorElement_: HTMLElement|null;
-  private showDialog_: boolean;
+  private showEditDialog_: boolean;
+  private showDeleteConfirmationDialog_: boolean;
   private showKeywordTriggerSetting_: boolean;
   private isActiveSearchEnginesFlagEnabled_: boolean;
   private browserProxy_: SearchEnginesBrowserProxy =
@@ -178,24 +190,45 @@ export class SettingsSearchEnginesPageElement extends
     this.addEventListener(
         'edit-search-engine',
         e => this.onEditSearchEngine_(e as SearchEngineEditEvent));
+
+    this.addEventListener(
+        'delete-search-engine',
+        e => this.onDeleteSearchEngine_(e as SearchEngineDeleteEvent));
   }
 
-  private openDialog_(
+  private openEditDialog_(
       searchEngine: SearchEngine|null, anchorElement: HTMLElement) {
     this.dialogModel_ = searchEngine;
     this.dialogAnchorElement_ = anchorElement;
-    this.showDialog_ = true;
+    this.showEditDialog_ = true;
   }
 
-  private onCloseDialog_() {
-    this.showDialog_ = false;
+  private openDeleteConfirmationDialog_(
+      searchEngine: SearchEngine|null, anchorElement: HTMLElement) {
+    this.dialogModel_ = searchEngine;
+    this.dialogAnchorElement_ = anchorElement;
+    this.showDeleteConfirmationDialog_ = true;
+  }
+
+  private onCloseEditDialog_() {
+    this.showEditDialog_ = false;
     focusWithoutInk(this.dialogAnchorElement_ as HTMLElement);
     this.dialogModel_ = null;
     this.dialogAnchorElement_ = null;
   }
 
+  private onCloseDeleteConfirmationDialog_() {
+    this.showDeleteConfirmationDialog_ = false;
+    this.dialogModel_ = null;
+    this.dialogAnchorElement_ = null;
+  }
+
   private onEditSearchEngine_(e: SearchEngineEditEvent) {
-    this.openDialog_(e.detail.engine, e.detail.anchorElement);
+    this.openEditDialog_(e.detail.engine, e.detail.anchorElement);
+  }
+
+  private onDeleteSearchEngine_(e: SearchEngineDeleteEvent) {
+    this.openDeleteConfirmationDialog_(e.detail.engine, e.detail.anchorElement);
   }
 
   private extensionsChanged_() {
@@ -220,7 +253,7 @@ export class SettingsSearchEnginesPageElement extends
 
   private onAddSearchEngineTap_(e: Event) {
     e.preventDefault();
-    this.openDialog_(null, this.$.addSearchEngine);
+    this.openEditDialog_(null, this.$.addSearchEngine);
   }
 
   private computeShowExtensionsList_(): boolean {

chrome/browser/resources/settings/settings.gni

@@ -122,7 +122,8 @@ web_component_files = [
   "safety_check_page/safety_check_updates_child.ts",
   "search_page/search_page.ts",
   "search_engines_page/omnibox_extension_entry.ts",
-  "search_engines_page/search_engine_dialog.ts",
+  "search_engines_page/search_engine_delete_confirmation_dialog.ts",
+  "search_engines_page/search_engine_edit_dialog.ts",
   "search_engines_page/search_engine_entry_css.ts",
   "search_engines_page/search_engine_entry.ts",
   "search_engines_page/search_engines_list.ts",

chrome/browser/ui/search_engines/keyword_editor_controller.cc

@@ -82,6 +82,13 @@ bool KeywordEditorController::CanDeactivate(const TemplateURL* url) const {
           url->prepopulate_id() == 0);
 }
 
+bool KeywordEditorController::ShouldConfirmDeletion(
+    const TemplateURL* url) const {
+  // Currently, only built-in search engines require confirmation before
+  // deletion.
+  return url->prepopulate_id() != 0;
+}
+
 void KeywordEditorController::RemoveTemplateURL(int index) {
   table_model_->Remove(index);
   base::RecordAction(UserMetricsAction("KeywordEditor_RemoveKeyword"));

chrome/browser/ui/search_engines/keyword_editor_controller.h

@@ -58,6 +58,10 @@ class KeywordEditorController {
   // the current default search engine.
   bool CanDeactivate(const TemplateURL* url) const;
 
+  // Return true if the user should be asked to confirm before deleting the
+  // given `url`.
+  bool ShouldConfirmDeletion(const TemplateURL* url) const;
+
   // Remove the TemplateURL at the specified index in the TableModel.
   void RemoveTemplateURL(int index);
 

chrome/browser/ui/webui/settings/search_engines_handler.cc

@@ -33,7 +33,7 @@
 
 namespace {
 // The following strings need to match with the IDs of the text input elements
-// at settings/search_engines_page/search_engine_dialog.html.
+// at settings/search_engines_page/search_engine_edit_dialog.html.
 const char kSearchEngineField[] = "searchEngine";
 const char kKeywordField[] = "keyword";
 const char kQueryUrlField[] = "queryUrl";
@@ -227,6 +227,8 @@ SearchEnginesHandler::CreateDictionaryForEngine(int index, bool is_default) {
                    list_controller_.CanActivate(template_url));
   dict->SetBoolean("canBeDeactivated",
                    list_controller_.CanDeactivate(template_url));
+  dict->SetBoolean("shouldConfirmDeletion",
+                   list_controller_.ShouldConfirmDeletion(template_url));
   TemplateURL::Type type = template_url->type();
   dict->SetBoolean("isOmniboxExtension",
                    type == TemplateURL::OMNIBOX_API_EXTENSION);