From 24abed13369469c174acbec5e5939e252b29936f Mon Sep 17 00:00:00 2001
From: "Patrick H. Lauke" <redux@splintered.co.uk>
Date: Wed, 5 Jun 2019 14:23:25 +0100
Subject: Fix keyboard handling of button-style checkbox/radio button groups
 (#28834)

- adds more defensive checks to make sure no unnecessary toggling happens on disabled buttons; this also fixes an up-to-now undiscovered bug where a toggle button with `.disabled` class would still have its `aria-pressed` toggled
- adds a set of explicit tests for the above case of disabled buttons and `aria-pressed`
- remove a now irrelevant (or at least very nonsensical) test for `<label>` containing both an actionable and a `hidden` `<input>`
- expand the test for disabled checkbox to also explicitly test starting conditions (used mainly in my debugging)
- ensure that `$btn[0].click()` is used to click checkboxes in tests, rather than the `click()` on the jquery object which is simply a shorthand for `trigger('click')` and does not actually trigger the browser default behavior
- remove the `preventDefault()` from the button handling, which was preventing correct keyboard functionality for checkboxes/radio buttons
- add extra logic to the button.js code to handle checkboxes correctly and avoid double-triggering as a result of mouse interactions (which saw the checkboxes being toggled twice, thus returning them to their original state)
- add logic that prevents the `checked` property from being added incorrectly for any inputs other than radio buttons and checkboxes
- added more tests (including the most basic test for a properly triggered fake checkbox button)
- work around Firefox bug #1540995 (which this code was hitting after removing the `preventDefault()`, due to Firefox's incorrect toggling of disabled checkboxes when programmatically (but not manually) activated with a `click()` event
---
 js/src/button.js | 44 ++++++++++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 16 deletions(-)

(limited to 'js/src')

diff --git a/js/src/button.js b/js/src/button.js
index fcf805502a..6225137019 100644
--- a/js/src/button.js
+++ b/js/src/button.js
@@ -81,15 +81,16 @@ class Button {
               $(activeElement).removeClass(ClassName.ACTIVE)
             }
           }
+        } else if (input.type === 'checkbox') {
+          if (this._element.tagName === 'LABEL' && input.checked === this._element.classList.contains(ClassName.ACTIVE)) {
+            triggerChangeEvent = false
+          }
+        } else {
+          // if it's not a radio button or checkbox don't add a pointless/invalid checked property to the input
+          triggerChangeEvent = false
         }
 
         if (triggerChangeEvent) {
-          if (input.hasAttribute('disabled') ||
-            rootElement.hasAttribute('disabled') ||
-            input.classList.contains('disabled') ||
-            rootElement.classList.contains('disabled')) {
-            return
-          }
           input.checked = !this._element.classList.contains(ClassName.ACTIVE)
           $(input).trigger('change')
         }
@@ -99,13 +100,15 @@ class Button {
       }
     }
 
-    if (addAriaPressed) {
-      this._element.setAttribute('aria-pressed',
-        !this._element.classList.contains(ClassName.ACTIVE))
-    }
+    if (!(this._element.hasAttribute('disabled') || this._element.classList.contains('disabled'))) {
+      if (addAriaPressed) {
+        this._element.setAttribute('aria-pressed',
+          !this._element.classList.contains(ClassName.ACTIVE))
+      }
 
-    if (triggerChangeEvent) {
-      $(this._element).toggleClass(ClassName.ACTIVE)
+      if (triggerChangeEvent) {
+        $(this._element).toggleClass(ClassName.ACTIVE)
+      }
     }
   }
 
@@ -140,15 +143,24 @@ class Button {
 
 $(document)
   .on(Event.CLICK_DATA_API, Selector.DATA_TOGGLE_CARROT, (event) => {
-    event.preventDefault()
-
     let button = event.target
 
     if (!$(button).hasClass(ClassName.BUTTON)) {
-      button = $(button).closest(Selector.BUTTON)
+      button = $(button).closest(Selector.BUTTON)[0]
     }
 
-    Button._jQueryInterface.call($(button), 'toggle')
+    if (!button || button.hasAttribute('disabled') || button.classList.contains('disabled')) {
+      event.preventDefault() // work around Firefox bug #1540995
+    } else {
+      const inputBtn = button.querySelector(Selector.INPUT)
+
+      if (inputBtn && (inputBtn.hasAttribute('disabled') || inputBtn.classList.contains('disabled'))) {
+        event.preventDefault() // work around Firefox bug #1540995
+        return
+      }
+
+      Button._jQueryInterface.call($(button), 'toggle')
+    }
   })
   .on(Event.FOCUS_BLUR_DATA_API, Selector.DATA_TOGGLE_CARROT, (event) => {
     const button = $(event.target).closest(Selector.BUTTON)[0]
-- 
cgit v1.2.3