Fix generated options not receiving result IDs (#5586)

In order to enable the ability to uniquely identify a result by an ID
in the DOM, we generate a new ID for the result based on a combination
of things, including the container ID prefix that is generated and
used elsewhere in Select2. This has worked fairly well for use cases
including attaching Select2 to an existing `<select>` and loading in
options from a remote data set.

Unfortunately, because this process relied on the container ID being
used as a prefix, this failed for options which were automatically
generated on initialization using the `data:` option to Select2.
These were not being generated with an ID because at the time that
they were being generated, the data adapter was not aware of the
container it was being used in. This broke some accessibility features
because we had a mix of options in the results list with IDs, and
some without, so we fixed the ordering to make this work.

Option generation no longer happens when the data adapter is first
initialized, which is where it was previously happening, and instead
it now occurs when the data adapter is bound to the container. This
allows us to ensure that the data adapter is always aware of the
container it is being associated with, so now it will be able to
generate the result IDs.

This also fixes the tests for the array adapter as well as the
legacy `<input />` adapter so they properly bind to a container
during the test. This was causing test failures becuase the options
which would previously be generated during initialization were no
longer appearing.

Fixes #4350

src/js/select2/data/array.js

@@ -4,15 +4,19 @@ define([
   'jquery'
 ], function (SelectAdapter, Utils, $) {
   function ArrayAdapter ($element, options) {
-    var data = options.get('data') || [];
+    this._dataToConvert = options.get('data') || [];
 
     ArrayAdapter.__super__.constructor.call(this, $element, options);
-
-    this.addOptions(this.convertToOptions(data));
   }
 
   Utils.Extend(ArrayAdapter, SelectAdapter);
 
+  ArrayAdapter.prototype.bind = function (container, $container) {
+    ArrayAdapter.__super__.bind.call(this, container, $container);
+
+    this.addOptions(this.convertToOptions(this._dataToConvert));
+  };
+
   ArrayAdapter.prototype.select = function (data) {
     var $option = this.$element.find('option').filter(function (i, elm) {
       return elm.value == data.id.toString();

tests/data/array-tests.js

@@ -71,6 +71,9 @@ test('current gets default for single', function (assert) {
 
   var data = new ArrayData($select, arrayOptions);
 
+  var container = new MockContainer();
+  data.bind(container, $('<div></div>'));
+
   data.current(function (val) {
     assert.equal(
       val.length,
@@ -93,6 +96,9 @@ test('current gets default for multiple', function (assert) {
 
   var data = new ArrayData($select, arrayOptions);
 
+  var container = new MockContainer();
+  data.bind(container, $('<div></div>'));
+
   data.current(function (val) {
     assert.equal(
       val.length,
@@ -107,6 +113,9 @@ test('current works with existing selections', function (assert) {
 
   var data = new ArrayData($select, arrayOptions);
 
+  var container = new MockContainer();
+  data.bind(container, $('<div></div>'));
+
   $select.val(['One']);
 
   data.current(function (val) {
@@ -137,6 +146,9 @@ test('current works with selected data', function (assert) {
 
   var data = new ArrayData($select, arrayOptions);
 
+  var container = new MockContainer();
+  data.bind(container, $('<div></div>'));
+
   data.select({
     id: '2',
     text: '2'
@@ -170,6 +182,9 @@ test('select works for single', function (assert) {
 
   var data = new ArrayData($select, arrayOptions);
 
+  var container = new MockContainer();
+  data.bind(container, $('<div></div>'));
+
   assert.equal(
     $select.val(),
     'default',
@@ -193,6 +208,9 @@ test('multiple sets the value', function (assert) {
 
   var data = new ArrayData($select, arrayOptions);
 
+  var container = new MockContainer();
+  data.bind(container, $('<div></div>'));
+
   assert.ok(
     $select.val() == null || $select.val().length == 0,
     'nothing should be selected'
@@ -211,6 +229,9 @@ test('multiple adds to the old value', function (assert) {
 
   var data = new ArrayData($select, arrayOptions);
 
+  var container = new MockContainer();
+  data.bind(container, $('<div></div>'));
+
   $select.val(['One']);
 
   assert.deepEqual($select.val(), ['One']);
@@ -228,18 +249,42 @@ test('option tags are automatically generated', function (assert) {
 
   var data = new ArrayData($select, arrayOptions);
 
+  var container = new MockContainer();
+  data.bind(container, $('<div></div>'));
+
   assert.equal(
     $select.find('option').length,
     4,
     'An <option> element should be created for each object'
   );
 });
 
+test('automatically generated option tags have a result id', function (assert) {
+  var $select = $('#qunit-fixture .single-empty');
+
+  var data = new ArrayData($select, arrayOptions);
+
+  var container = new MockContainer();
+  data.bind(container, $('<div></div>'));
+
+  data.select({
+    id: 'default'
+  });
+
+  assert.ok(
+    Utils.GetData($select.find(':selected')[0], 'data')._resultId,
+    '<option> default should have a result ID assigned'
+  );
+});
+
 test('option tags can receive new data', function(assert) {
   var $select = $('#qunit-fixture .single');
 
   var data = new ArrayData($select, extraOptions);
 
+  var container = new MockContainer();
+  data.bind(container, $('<div></div>'));
+
   assert.equal(
     $select.find('option').length,
     2,
@@ -270,6 +315,9 @@ test('optgroup tags can also be generated', function (assert) {
 
   var data = new ArrayData($select, nestedOptions);
 
+  var container = new MockContainer();
+  data.bind(container, $('<div></div>'));
+
   assert.equal(
     $select.find('option').length,
     1,
@@ -288,6 +336,9 @@ test('optgroup tags have the right properties', function (assert) {
 
   var data = new ArrayData($select, nestedOptions);
 
+  var container = new MockContainer();
+  data.bind(container, $('<div></div>'));
+
   var $group = $select.children('optgroup');
 
   assert.equal(
@@ -328,5 +379,8 @@ test('existing selections are respected on initialization', function (assert) {
 
     var data = new ArrayData($select, options);
 
+    var container = new MockContainer();
+    data.bind(container, $('<div></div>'));
+
     assert.equal($select.val(), 'Second');
 });

tests/data/inputData-tests.js

@@ -23,6 +23,9 @@ test('test that options can be selected', function (assert) {
 
   var adapter = new InputAdapter($element, options);
 
+  var container = new MockContainer();
+  adapter.bind(container, $('<div></div>'));
+
   adapter.select({
     id: 'test'
   });
@@ -48,6 +51,9 @@ test('unselect the single selected option clears the value', function (assert) {
 
   var adapter = new InputAdapter($element, options);
 
+  var container = new MockContainer();
+  adapter.bind(container, $('<div></div>'));
+
   adapter.unselect({
     id: 'test'
   });
@@ -81,6 +87,9 @@ test('options can be unselected individually', function (assert) {
 
   var adapter = new InputAdapter($element, options);
 
+  var container = new MockContainer();
+  adapter.bind(container, $('<div></div>'));
+
   adapter.unselect({
     id: 'test2'
   });
@@ -107,6 +116,9 @@ test('default values can be set', function (assert) {
 
   var adapter = new InputAdapter($element, options);
 
+  var container = new MockContainer();
+  adapter.bind(container, $('<div></div>'));
+
   adapter.current(function (data) {
     assert.equal(
       data.length,
@@ -142,6 +154,9 @@ test('no default value', function (assert) {
 
   var adapter = new InputAdapter($element, options);
 
+  var container = new MockContainer();
+  adapter.bind(container, $('<div></div>'));
+
   adapter.current(function (data) {
     assert.equal(
       data.length,