Merge pull request #101 from GoogleChrome/focus-nonmodal

focus on show() as well as showModal()

dialog-polyfill.js

@@ -159,6 +159,28 @@
       e.stopPropagation();
     },
 
+    /**
+     * Focuses on the first focusable element within the dialog. This will always blur the current
+     * focus, even if nothing within the dialog is found.
+     */
+    focus_: function() {
+      // Find element with `autofocus` attribute, or fall back to the first form/tabindex control.
+      var target = this.dialog_.querySelector('[autofocus]:not([disabled])');
+      if (!target) {
+        // Note that this is 'any focusable area'. This list is probably not exhaustive, but the
+        // alternative involves stepping through and trying to focus everything.
+        var opts = ['button', 'input', 'keygen', 'select', 'textarea'];
+        var query = opts.map(function(el) {
+          return el + ':not([disabled])';
+        });
+        // TODO(samthor): tabindex values that are not numeric are not focusable.
+        query.push('[tabindex]:not([disabled]):not([tabindex=""])');  // tabindex != "", not disabled
+        target = this.dialog_.querySelector(query.join(', '));
+      }
+      safeBlur(document.activeElement);
+      target && target.focus();
+    },
+
     /**
      * Sets the zIndex for the backdrop and dialog.
      *
@@ -171,10 +193,13 @@
     },
 
     /**
-     * Shows the dialog. This is idempotent and will always succeed.
+     * Shows the dialog. If the dialog is already open, this does nothing.
      */
     show: function() {
-      this.setOpen(true);
+      if (!this.dialog_.open) {
+        this.setOpen(true);
+        this.focus_();
+      }
     },
 
     /**
@@ -205,19 +230,6 @@
       this.backdrop_.addEventListener('click', this.backdropClick_);
       this.dialog_.parentNode.insertBefore(this.backdrop_,
           this.dialog_.nextSibling);
-
-      // Find element with `autofocus` attribute or first form control.
-      var target = this.dialog_.querySelector('[autofocus]:not([disabled])');
-      if (!target) {
-        // TODO: technically this is 'any focusable area'
-        var opts = ['button', 'input', 'keygen', 'select', 'textarea'];
-        var query = opts.map(function(el) {
-          return el + ':not([disabled])';
-        }).join(', ');
-        target = this.dialog_.querySelector(query);
-      }
-      safeBlur(document.activeElement);
-      target && target.focus();
     },
 
     /**

suite.js

@@ -269,16 +269,6 @@ void function() {
   });
 
   suite('form focus', function() {
-    test('no focus change on non-modal', function() {
-      var input = cleanup(document.createElement('input'));
-      input.type = 'text';
-      dialog.appendChild(input);
-
-      var previous = document.activeElement;
-      dialog.show();
-      assert.equal(document.activeElement, previous);
-      dialog.close();
-    });
     test('clear focus when nothing focusable in modal', function() {
       var input = cleanup(document.createElement('input'));
       input.type = 'text';
@@ -301,6 +291,14 @@ void function() {
       dialog.showModal();
       assert.equal(document.activeElement, input);
     });
+    test('default focus on non-modal', function() {
+      var div = cleanup(document.createElement('div'));
+      div.tabIndex = 4;
+      dialog.appendChild(div);
+
+      dialog.show();
+      assert.equal(document.activeElement, div);
+    });
     test('autofocus element chosen', function() {
       var input = cleanup(document.createElement('input'));
       input.type = 'text';
@@ -344,8 +342,8 @@ void function() {
       input.focus();
 
       dialog.show();
-      assert.equal(document.activeElement, input,
-          'non-modal dialog shouldn\'t clear focus');
+      assert.notEqual(document.activeElement, input,
+        'non-modal dialog should clear focus, even with no dialog content');
 
       document.body.focus();
       input.focus();