Prevent opening of disabled elements (select2#5751)

mcountis · kevin-brown · kevin-brown · commit f34c84b77f0d · 2020-01-27T23:41:23.000-05:00
* Prevent a disabled element from opening via keydown/keypress events

Gist: return early from the Select2#open method if the object is
disabled.

Add helper #isEnabled and #isDisabled methods to Select2 and selector
classes.

Add test coverage for the original, expected behavior and new
preventative behavior.

* Remove single-use method from tests

Co-authored-by: Kevin Brown &lt;kevin-brown@users.noreply.github.com&gt;
diff --git a/src/js/select2/core.js b/src/js/select2/core.js
@@ -365,7 +365,7 @@ define([
   Select2.prototype._syncAttributes = function () {
     this.options.set('disabled', this.$element.prop('disabled'));
 
-    if (this.options.get('disabled')) {
+    if (this.isDisabled()) {
       if (this.isOpen()) {
         this.close();
       }
@@ -470,7 +470,7 @@ define([
   };
 
   Select2.prototype.toggleDropdown = function () {
-    if (this.options.get('disabled')) {
+    if (this.isDisabled()) {
       return;
     }
 
@@ -486,6 +486,10 @@ define([
       return;
     }
 
+    if (this.isDisabled()) {
+      return;
+    }
+
     this.trigger('query', {});
   };
 
@@ -497,6 +501,27 @@ define([
     this.trigger('close', { originalEvent : evt });
   };
 
+  /**
+   * Helper method to abstract the "enabled" (not "disabled") state of this
+   * object.
+   *
+   * @return {true} if the instance is not disabled.
+   * @return {false} if the instance is disabled.
+   */
+  Select2.prototype.isEnabled = function () {
+    return !this.isDisabled();
+  };
+
+  /**
+   * Helper method to abstract the "disabled" state of this object.
+   *
+   * @return {true} if the disabled option is true.
+   * @return {false} if the disabled option is false.
+   */
+  Select2.prototype.isDisabled = function () {
+    return this.options.get('disabled');
+  };
+
   Select2.prototype.isOpen = function () {
     return this.$container.hasClass('select2-container--open');
   };
diff --git a/src/js/select2/selection/allowClear.js b/src/js/select2/selection/allowClear.js
@@ -31,7 +31,7 @@ define([
 
   AllowClear.prototype._handleClear = function (_, evt) {
     // Ignore the event if it is disabled
-    if (this.options.get('disabled')) {
+    if (this.isDisabled()) {
       return;
     }
 
diff --git a/src/js/select2/selection/base.js b/src/js/select2/selection/base.js
@@ -153,5 +153,26 @@ define([
     throw new Error('The `update` method must be defined in child classes.');
   };
 
+  /**
+   * Helper method to abstract the "enabled" (not "disabled") state of this
+   * object.
+   *
+   * @return {true} if the instance is not disabled.
+   * @return {false} if the instance is disabled.
+   */
+  BaseSelection.prototype.isEnabled = function () {
+    return !this.isDisabled();
+  };
+
+  /**
+   * Helper method to abstract the "disabled" state of this object.
+   *
+   * @return {true} if the disabled option is true.
+   * @return {false} if the disabled option is false.
+   */
+  BaseSelection.prototype.isDisabled = function () {
+    return this.options.get('disabled');
+  };
+
   return BaseSelection;
 });
diff --git a/src/js/select2/selection/multiple.js b/src/js/select2/selection/multiple.js
@@ -37,7 +37,7 @@ define([
       '.select2-selection__choice__remove',
       function (evt) {
         // Ignore the event if it is disabled
-        if (self.options.get('disabled')) {
+        if (self.isDisabled()) {
           return;
         }
 
diff --git a/tests/selection/openOnKeyDown-tests.js b/tests/selection/openOnKeyDown-tests.js
@@ -0,0 +1,188 @@
+module('Selection containers - Open On Key Down');
+
+var KEYS = require('select2/keys');
+var $ = require('jquery');
+
+/**
+ * Build a keydown event with the given key code and extra options.
+ *
+ * @param  {Number} keyCode     the keyboard code to be used for the 'which'
+ *                              attribute of the keydown event.
+ * @param  {Object} eventProps  extra properties to build the keydown event.
+ *
+ * @return {jQuery.Event} a 'keydown' type event.
+ */
+function buildKeyDownEvent (keyCode, eventProps) {
+  return $.Event('keydown', $.extend({}, { which: keyCode }, eventProps));
+}
+
+/**
+ * Wrapper function providing a select2 element with a given enabled/disabled
+ * state that will get a given keydown event triggered on it. Provide an
+ * assertion callback function to test the results of the triggered event.
+ *
+ * @param  {Boolean}  isEnabled   the enabled state of the desired select2
+ *                                element.
+ * @param  {String}   testName    name for the test.
+ * @param  {Number}   keyCode     used to set the 'which' attribute of the
+ *                                keydown event.
+ * @param  {Object}   eventProps  attributes to be used to build the keydown
+ *                                event.
+ * @param  {Function} fn          assertion callback to perform checks on the
+ *                                result of triggering the event, receives the
+ *                                'assert' variable for the test and the select2
+ *                                instance behind the built <select> element.
+ * @return {null}
+ */
+function testAbled(isEnabled, testName, keyCode, eventProps, fn) {
+  test(testName, function (assert) {
+    var $element = $(
+      '<select>' +
+        '<option>one</option>' +
+        '<option>two</option>' +
+      '</select>'
+    );
+    $('#qunit-fixture').append($element);
+    $element.select2({ disabled: !isEnabled });
+
+    var select2 = $element.data('select2');
+    var $selection = select2.$selection;
+
+    assert.notOk(select2.isOpen(), 'The instance should not be open');
+    assert.equal(select2.isEnabled(), isEnabled);
+
+    var event = buildKeyDownEvent(keyCode, eventProps);
+    assert.ok(event.which, 'The event\'s key code (.which) should be set');
+
+    $selection.trigger(event);
+
+    fn(assert, select2);
+  });
+}
+
+/**
+ * Test the given keydown event on an enabled element. See #testAbled for
+ * params.
+ */
+function testEnabled (testName, keyCode, eventProps, fn) {
+  testAbled(true, testName, keyCode, eventProps, fn);
+}
+
+/**
+ * Test the given keydown event on a disabled element. See #testAbled for
+ * params.
+ */
+function testDisabled (testName, keyCode, eventProps, fn) {
+  testAbled(false, testName, keyCode, eventProps, fn);
+}
+
+/**
+ * Assertion function used by the above test* wrappers. Asserts that the given
+ * select2 instance is open.
+ *
+ * @param  {Assert}   assert
+ * @param  {Select2}  select
+ * @return {null}
+ */
+function assertOpened (assert, select2) {
+  assert.ok(select2.isOpen(), 'The element should be open');
+}
+
+/**
+ * Assertion function used by the above test* wrappers. Asserts that the given
+ * select2 instance is not open.
+ *
+ * @param  {Assert}   assert
+ * @param  {Select2}  select
+ * @return {null}
+ */
+function assertNotOpened (assert, select2) {
+  assert.notOk(select2.isOpen(), 'The element should not be open');
+}
+
+/**
+ * ENTER, SPACE, and ALT+DOWN should all open an enabled select2 element.
+ */
+testEnabled(
+  'enabled element will open on ENTER',
+  KEYS.ENTER, {},
+  assertOpened
+);
+testEnabled(
+  'enabled element will open on SPACE',
+  KEYS.SPACE, {},
+  assertOpened
+);
+testEnabled(
+  'enabled element will open on ALT+DOWN',
+  KEYS.DOWN, { altKey: true },
+  assertOpened
+);
+
+/**
+ * Some other keys triggered on an enabled select2 element should not open it.
+ */
+testEnabled(
+  'enabled element will not open on UP',
+  KEYS.UP, {},
+  assertNotOpened
+);
+testEnabled(
+  'enabled element will not open on DOWN',
+  KEYS.UP, {},
+  assertNotOpened
+);
+testEnabled(
+  'enabled element will not open on LEFT',
+  KEYS.UP, {},
+  assertNotOpened
+);
+testEnabled(
+  'enabled element will not open on RIGHT',
+  KEYS.UP, {},
+  assertNotOpened
+);
+
+/*
+ * The keys that will open an enabled select2 element should not open a disabled
+ * one.
+ */
+testDisabled(
+  'disabled element will not open on ENTER',
+  KEYS.ENTER, {},
+  assertNotOpened
+);
+testDisabled(
+  'disabled element will not open on SPACE',
+  KEYS.SPACE, {},
+  assertNotOpened
+);
+testDisabled(
+  'disabled element will not open on ALT+DOWN',
+  KEYS.DOWN, { altKey: true },
+  assertNotOpened
+);
+
+/**
+ * Other keys should continue to not open a disabled select2 element.
+ */
+testDisabled(
+  'disabled element will not open on UP',
+  KEYS.UP, {},
+  assertNotOpened
+);
+testDisabled(
+  'disabled element will not open on DOWN',
+  KEYS.UP, {},
+  assertNotOpened
+);
+testDisabled(
+  'disabled element will not open on LEFT',
+  KEYS.UP, {},
+  assertNotOpened
+);
+testDisabled(
+  'disabled element will not open on RIGHT',
+  KEYS.UP, {},
+  assertNotOpened
+);
diff --git a/tests/unit-jq1.html b/tests/unit-jq1.html
@@ -97,6 +97,7 @@
     <script src="selection/search-placeholder-tests.js" type="text/javascript"></script>
     <script src="selection/single-tests.js" type="text/javascript"></script>
     <script src="selection/stopPropagation-tests.js" type="text/javascript"></script>
+    <script src="selection/openOnKeyDown-tests.js" type="text/javascript"></script>
 
     <script src="utils/data-tests.js" type="text/javascript"></script>
     <script src="utils/decorator-tests.js" type="text/javascript"></script>
diff --git a/tests/unit-jq2.html b/tests/unit-jq2.html
@@ -97,6 +97,7 @@
     <script src="selection/search-placeholder-tests.js" type="text/javascript"></script>
     <script src="selection/single-tests.js" type="text/javascript"></script>
     <script src="selection/stopPropagation-tests.js" type="text/javascript"></script>
+    <script src="selection/openOnKeyDown-tests.js" type="text/javascript"></script>
 
     <script src="utils/data-tests.js" type="text/javascript"></script>
     <script src="utils/decorator-tests.js" type="text/javascript"></script>
diff --git a/tests/unit-jq3.html b/tests/unit-jq3.html
@@ -97,6 +97,7 @@
     <script src="selection/search-placeholder-tests.js" type="text/javascript"></script>
     <script src="selection/single-tests.js" type="text/javascript"></script>
     <script src="selection/stopPropagation-tests.js" type="text/javascript"></script>
+    <script src="selection/openOnKeyDown-tests.js" type="text/javascript"></script>
 
     <script src="utils/data-tests.js" type="text/javascript"></script>
     <script src="utils/decorator-tests.js" type="text/javascript"></script>

Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ define([`
`31`	`31`
`32`	`32`	`AllowClear.prototype._handleClear = function (_, evt) {`
`33`	`33`	`// Ignore the event if it is disabled`
`34`		`- if (this.options.get('disabled')) {`
	`34`	`+ if (this.isDisabled()) {`
`35`	`35`	`return;`
`36`	`36`	`}`
`37`	`37`
Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,7 @@ define([`
`37`	`37`	`'.select2-selection__choice__remove',`
`38`	`38`	`function (evt) {`
`39`	`39`	`// Ignore the event if it is disabled`
`40`		`- if (self.options.get('disabled')) {`
	`40`	`+ if (self.isDisabled()) {`
`41`	`41`	`return;`
`42`	`42`	`}`
`43`	`43`