WebUI: For cr-dialog, make Enter key ignore disabled or hidden buttons

chrome/test/data/webui/cr_elements/cr_dialog_test.js

 // Copyright 2016 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.
 
 suite('cr-dialog', function() {
   setup(function() {
     PolymerTest.clearBody();
   });
 
   test('focuses title on show', function() {
@@ skipping 29 matching lines @@
     var actionButton = document.body.querySelector('.action-button');
 
     dialog.showModal();
 
     // MockInteractions triggers event listeners synchronously.
     var clickedCounter = 0;
     actionButton.addEventListener('click', function() {
       clickedCounter += 1;
     });
 
     // Enter key should ignore disabled buttons.

[dpapad, 2017/04/04 17:10:17]

Are lines 50-57 now redundant, meaning that the new test already covers those
cases? If so, perhaps consider removing from here.

[tommycli, 2017/04/04 17:44:10]

Done.

     MockInteractions.keyEventOn(dialog, 'keypress', 13, undefined, 'Enter');
     assertEquals(0, clickedCounter);
 
     // Enter key should trigger enabled buttons.
     actionButton.disabled = false;
     MockInteractions.keyEventOn(dialog, 'keypress', 13, undefined, 'Enter');
     assertEquals(1, clickedCounter);
 
     // Enter keys on other buttons should be ignored.
     clickedCounter = 0;
     var otherButton = document.body.querySelector('#other-button');
     assertTrue(!!otherButton);
     MockInteractions.keyEventOn(
         otherButton, 'keypress', 13, undefined, 'Enter');
     assertEquals(0, clickedCounter);
 
     // Enter key on the action button should only fire the click handler once.
     MockInteractions.tap(actionButton, 'keypress', 13, undefined, 'Enter');
     assertEquals(1, clickedCounter);
   });
 
+  test('enter ignores hidden and disabled action buttons', function() {
+    document.body.innerHTML = `
+      <dialog is="cr-dialog">
+        <div class="title">title</div>
+        <div class="body">
+          <button id="hidden" class="action-button" hidden>hidden</button>
+          <button class="action-button" disabled>disabled</button>
+          <button class="action-button" disabled hidden>disabled hidden</button>
+          <button id="active" class="action-button">active</button>
+        </div>
+      </dialog>`;
+
+    var dialog = document.body.querySelector('dialog');
+    var hiddenButton = document.body.querySelector('#hidden');
+    var actionButton = document.body.querySelector('#active');
+    dialog.showModal();
+
+    // MockInteractions triggers event listeners synchronously.
+    hiddenButton.addEventListener('click', function() {
+      throw new Error('Hidden button received a click.');

[dpapad, 2017/04/04 17:10:17]

Nit: Can we use assertNotReached() instead? We actually have two such functions
and (unfortunately) whichever gets defined last overwrites the previous
definitions (see links below).

https://cs.chromium.org/chromium/src/chrome/test/data/webui/test_api.js?q&l=940
https://cs.chromium.org/chromium/src/ui/webui/resources/js/assert.js?l=55

[tommycli, 2017/04/04 17:44:11]

Done. And I can confirm this fails the test if I remove the :not([hidden])
selector qualifier.

+    })
+    var clicked = false;
+    actionButton.addEventListener('click', function() {
+      clicked = true;
+    });
+
+    MockInteractions.keyEventOn(dialog, 'keypress', 13, undefined, 'Enter');
+    assertTrue(clicked);
+  });
+
   test('focuses [autofocus] instead of title when present', function() {
     document.body.innerHTML = `
       <dialog is="cr-dialog">
         <div class="title">title</div>
         <div class="body"><button autofocus>button</button></div>
       </dialog>`;
 
     var dialog = document.body.querySelector('dialog');
     var button = document.body.querySelector('button');
 
@@ skipping 33 matching lines @@
       }
     });
     observer.observe(bodyContainer, {attributes: true});
 
     // Height is normally set via CSS, but mixin doesn't work with innerHTML.
     bodyContainer.style.height = '1px';
     bodyContainer.scrollTop = 100;
     dialog.showModal();  // Attach the dialog for the first time here.
   });
 });