Fix event propagation from inactive and disabled dropdowns (Fixes twb…

…s#30510)
luktom · Apr 17, 2020 · a33c87f · a33c87f
1 parent a8883a3
commit a33c87f
Show file tree

Hide file tree

Showing 2 changed files with 96 additions and 17 deletions.
diff --git a/js/src/dropdown.js b/js/src/dropdown.js
@@ -434,30 +434,50 @@ class Dropdown {
     return getElementFromSelector(element) || element.parentNode
   }
 
+  // If not input/textarea:
+  //  - And not a key in REGEXP_KEYDOWN => not a dropdown command
+  //  - And dropdown disabled or not active and escape key => not a dropdown command
+  // If input/textarea:
+  //  - If space key => not a dropdown command
+  //  - If key is other than escape
+  //    - If key is not up or down => not a dropdown command
+  //    - If trigger inside the menu => not a dropdown command
+  static _isDropDownCommand(event, dropdownElement) {
+    const isInputOrTextArea = /input|textarea/i.test(event.target.tagName)
+    const isSpaceKeyEvent = event.key === SPACE_KEY
+    const isEscapeKeyEvent = event.key === ESCAPE_KEY
+    const isUpOrDownKeyEvent = event.key === ARROW_DOWN_KEY || event.key === ARROW_UP_KEY
+    const isTriggerInsideMenu = SelectorEngine.closest(event.target, SELECTOR_MENU)
+    const isKeyInRegexpKeydown = REGEXP_KEYDOWN.test(event.key)
+
+    const isActive = dropdownElement.classList.contains(CLASS_NAME_SHOW)
+    const isDisabled = dropdownElement.disabled || dropdownElement.classList.contains(CLASS_NAME_DISABLED)
+
+    if (isInputOrTextArea) {
+      if (isSpaceKeyEvent) {
+        return false
+      }
+
+      if (!isEscapeKeyEvent) {
+        return isUpOrDownKeyEvent && !isTriggerInsideMenu
+      }
+    } else if (isKeyInRegexpKeydown) {
+      if (isDisabled || (!isActive && isEscapeKeyEvent)) {
+        return false
+      }
+    }
+
+    return true
+  }
+
   static dataApiKeydownHandler(event) {
-    // If not input/textarea:
-    //  - And not a key in REGEXP_KEYDOWN => not a dropdown command
-    // If input/textarea:
-    //  - If space key => not a dropdown command
-    //  - If key is other than escape
-    //    - If key is not up or down => not a dropdown command
-    //    - If trigger inside the menu => not a dropdown command
-    if (/input|textarea/i.test(event.target.tagName) ?
-      event.key === SPACE_KEY || (event.key !== ESCAPE_KEY &&
-      ((event.key !== ARROW_DOWN_KEY && event.key !== ARROW_UP_KEY) ||
-        SelectorEngine.closest(event.target, SELECTOR_MENU))) :
-      !REGEXP_KEYDOWN.test(event.key)) {
+    if (!Dropdown._isDropDownCommand(event, this)) {
       return
     }
 
     event.preventDefault()
     event.stopPropagation()
 
-    if (this.disabled || this.classList.contains(CLASS_NAME_DISABLED)) {
-      return
-    }
-
-    const parent = Dropdown.getParentFromElement(this)
     const isActive = this.classList.contains(CLASS_NAME_SHOW)
 
     if (event.key === ESCAPE_KEY) {
@@ -472,6 +492,7 @@ class Dropdown {
       return
     }
 
+    const parent = Dropdown.getParentFromElement(this)
     const items = SelectorEngine.find(SELECTOR_VISIBLE_ITEMS, parent)
       .filter(isVisible)
 

diff --git a/js/tests/unit/dropdown.spec.js b/js/tests/unit/dropdown.spec.js
@@ -1557,6 +1557,64 @@ describe('Dropdown', () => {
         done()
       }, 20)
     })
+
+    it('should not stop key event propagation when dropdown is disabled', () => {
+      fixtureEl.innerHTML = [
+        '<div id="dropdown-container">',
+        '  <div class="dropdown">',
+        '    <button class="btn dropdown-toggle" id="toggle" data-toggle="dropdown" disabled>Dropdown</button>',
+        '    <div class="dropdown-menu">',
+        '      <a class="dropdown-item" id="item" href="#">Menu item</a>',
+        '    </div>',
+        '  </div>',
+        '</div>'
+      ]
+
+      const dropdownBtn = fixtureEl.querySelector('button[data-toggle="dropdown"]')
+      const dropDownContainer = fixtureEl.querySelector('#dropdown-container')
+      const eventHandlerSpy = jasmine.createSpy()
+
+      dropDownContainer.addEventListener('keydown', eventHandlerSpy)
+
+      const params = { bubbles: true, cancelable: false }
+      const keyDownEscape = createEvent('keydown', params)
+      keyDownEscape.key = 'Escape'
+
+      // Key escape
+      dropdownBtn.focus()
+      dropdownBtn.dispatchEvent(keyDownEscape)
+
+      expect(eventHandlerSpy).toHaveBeenCalled()
+    })
+
+    it('should not stop ESC key event propagation when dropdown is not active', () => {
+      fixtureEl.innerHTML = [
+        '<div id="dropdown-container">',
+        '  <div class="dropdown">',
+        '    <button class="btn dropdown-toggle" id="toggle" data-toggle="dropdown">Dropdown</button>',
+        '    <div class="dropdown-menu">',
+        '      <a class="dropdown-item" id="item" href="#">Menu item</a>',
+        '    </div>',
+        '  </div>',
+        '</div>'
+      ]
+
+      const dropdownBtn = fixtureEl.querySelector('button[data-toggle="dropdown"]')
+      const dropDownContainer = fixtureEl.querySelector('#dropdown-container')
+      const eventHandlerSpy = jasmine.createSpy()
+
+      dropDownContainer.addEventListener('keydown', eventHandlerSpy)
+
+      const params = { bubbles: true, cancelable: false }
+      const keyDownEscape = createEvent('keydown', params)
+      keyDownEscape.key = 'Escape'
+
+      // Key escape
+      dropdownBtn.focus()
+      dropdownBtn.dispatchEvent(keyDownEscape)
+
+      expect(eventHandlerSpy).toHaveBeenCalled()
+    })
   })
 
   describe('jQueryInterface', () => {