Make setting's protocol handler use cr-action-menu instead of paper-item.

chrome/browser/resources/settings/site_settings/protocol_handlers.html

Index: chrome/browser/resources/settings/site_settings/protocol_handlers.html
diff --git a/chrome/browser/resources/settings/site_settings/protocol_handlers.html b/chrome/browser/resources/settings/site_settings/protocol_handlers.html
index ab78ee849ffd445172499102b21594cc8b55584d..c16da4df24d7007d0e1fff9e7378581057773d84 100644
--- a/chrome/browser/resources/settings/site_settings/protocol_handlers.html
+++ b/chrome/browser/resources/settings/site_settings/protocol_handlers.html
@@ -1,10 +1,9 @@
 <link rel="import" href="chrome://resources/html/polymer.html">
 <link rel="import" href="chrome://resources/cr_elements/icons.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-icon-button/paper-icon-button.html">
-<link rel="import" href="chrome://resources/polymer/v1_0/paper-item/paper-item.html">
-<link rel="import" href="chrome://resources/polymer/v1_0/paper-menu/paper-menu.html">
-<link rel="import" href="chrome://resources/polymer/v1_0/paper-menu-button/paper-menu-button.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-toggle-button/paper-toggle-button.html">
+<link rel="import" href="chrome://resources/cr_elements/cr_action_menu/cr_action_menu.html">
+
 <link rel="import" href="chrome://resources/html/web_ui_listener_behavior.html">
 <link rel="import" href="/i18n_setup.html">
 <link rel="import" href="/settings_shared_css.html">
@@ -42,35 +41,34 @@
             <div class="favicon-image" style$="[[computeSiteIcon(item.host)]]">
             </div>
             <div class="middle" >
-              <div>[[item.host]]</div>
-              <div class="secondary"
+              <div class="protocol-host">[[item.host]]</div>
+              <div class="secondary protocol-default"
                   hidden$="[[!isDefault_(index, protocol.default_handler)]]">
                 $i18n{handlerIsDefault}
               </div>
             </div>
 
-            <paper-menu-button>
-              <paper-icon-button icon="cr:more-vert"
-                  class="dropdown-trigger">
-              </paper-icon-button>
-              <paper-menu id="actionMenu" class="dropdown-content" actionable
-                  on-iron-activate="onActionMenuIronActivate_"
-                  attr-for-selected="menu-value">
-                <paper-item menu-value$="[[menuActions_.SET_DEFAULT]]"
-                    hidden$="[[isDefault_(index, protocol.default_handler)]]"
-                    actionable>
-                  $i18n{handlerSetDefault}
-                </paper-item>
-                <paper-item menu-value$="[[menuActions_.REMOVE]]" actionable>
-                  $i18n{handlerRemove}
-                </paper-item>
-              </paper-menu>
-            </paper-menu-button>
+            <paper-icon-button icon="cr:more-vert" on-tap="showMenu_"
+                class="dropdown-trigger">
+            </paper-icon-button>
           </div>
 
         </template>
       </div>
     </template>
+
+    <dialog is="cr-action-menu">
+      <button class="dropdown-item" role="option" on-tap="onDefaultTap_"
+          hidden$="[[isDefault_(actionMenuModel_.index,

[dpapad, 2016/11/14 18:26:20]

I believe that hiding a menu option does not also make it non-focusable by
default which would break keyboard navigation within the menu (up/down arrows).
Can you verify if that's the case? Either way, I think we should also be
disabling hidden options, instead of just hiding them.

[Dan Beam, 2016/11/14 20:37:13]

stuff with a computed style of display: none; (which [hidden] does if specific
enough) wont be included in the focus/accessibility tree

[dpapad, 2016/11/14 20:39:34]

Looked it up after I added this comment. For the cr-action-menu case I think it
works because of this check
https://cs.chromium.org/chromium/src/ui/webui/resources/cr_elements/cr_action....

[scottchen, 2016/11/14 21:38:36]

The keyboard navigation seems to be working as expected for me.

+              actionMenuModel_.protocol.default_handler)]]"

[dpapad, 2016/11/14 18:26:20]

Can we simplify the isDefault_ data binding as follows?

// HTML
hidden$="isDefault(actionMenuModel_)"

// JS
isDefault_: function() {
  return this.actionMenuModel_.index ==
actionMenuModel_.protocol.default_handler;
}

[Dan Beam, 2016/11/14 20:37:13]

i think dpapad@ meant

hidden$="[[isDefault(actionMenuModel_)]]"

[scottchen, 2016/11/14 21:38:36]

Done. I created a new isModelDefault_ function because isDefault_ already exists
for rendering of the "Default" text in the dom-repeat above. 
Tried reusing the same function in both places, but there wasn't a way to access
"this model"  inside a dom-repeat render loop.

+          id="defaultButton">
+        $i18n{handlerSetDefault}
+      </button>
+      <button class="dropdown-item" role="option" on-tap="onRemoveTap_"
+          id="removeButton">
+        $i18n{handlerRemove}
+      </button>
+    </dialog>
   </template>
   <script src="protocol_handlers.js"></script>
 </dom-module>