Skip to content

Commit

Permalink
Restore compatibility with data-* attributes in jQuery 2.x (#5486)
Browse files Browse the repository at this point in the history
* Start running tests against jQuery 2.x

We were only running tests against jQuery 1.x before and they were
all passing. This was a problem because apparently all of the data-*
attribute tests fail in jQuery 2.x.  We are now running both the
integration and unit tests against both jQuery 1.x and jQuery 2.x.

Right now this resulted in a complete duplication of the test files
because there wasn't an obvious way to run the tests against both
versions. We're going to look into removing this duplication in the
future once the current issues are fixed.

We are also going to look into testing against jQuery 3.x in the
future, since that is also a supported line of jQuery.

* Force the data-* attributes to be parsed

There was a change made that switched us from using `$.data` and
`$.fn.data` internally to using an internal data store (managed
through internal utilities). This had the unfortunate side effect
of breaking the automatic loading of data-* options in versions of
jQuery other than 1.x, which included anything that would be
considered modern jQuery. While the change was made and approved
in good faith, all of the tests passed and the docs pages appeared
to be working, the tests really failed when running on newer versions
of jQuery. This was confirmed when we started running automated tests
against both versions, which confirmed the bug that others have been
seeing for a while.

The change was made becuase calling `$.fn.data` on an element which
contains a reference to itself in the internal jQuery data cache
would cause a stack overflow. This bug was well documented at the
following GitHub ticket and was resolved by no longer using
`$.fn.data`: #4014

Unfortunately because `$.fn.data` was no longer being called in a
way such that all of the data attributes would be dumped out, we
needed to find a replacement. The substitute that was given in the
original bug fix worked when the data cache was fully primed, but
we never primed it anywhere so it actually failed in the general
case. That meant we needed to find a way to manually prime it,
which is exactly what this change does.

* Clean up select2/utils
  • Loading branch information
kevin-brown authored Apr 28, 2019
1 parent 9f8b6ff commit 650035c
Show file tree
Hide file tree
Showing 7 changed files with 9,970 additions and 14 deletions.
33 changes: 28 additions & 5 deletions src/js/select2/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,20 +79,43 @@ define([

$e.attr('ajax--url', Utils.GetData($e[0], 'ajaxUrl'));
Utils.StoreData($e[0], 'ajax-Url', Utils.GetData($e[0], 'ajaxUrl'));

}

var dataset = {};

function upperCaseLetter(_, letter) {
return letter.toUpperCase();
}

// Pre-load all of the attributes which are prefixed with `data-`
for (var attr = 0; attr < $e[0].attributes.length; attr++) {
var attributeName = $e[0].attributes[attr].name;
var prefix = 'data-';

if (attributeName.substr(0, prefix.length) == prefix) {
// Get the contents of the attribute after `data-`
var dataName = attributeName.substring(prefix.length);

// Get the data contents from the consistent source
// This is more than likely the jQuery data helper
var dataValue = Utils.GetData($e[0], dataName);

// camelCase the attribute name to match the spec
var camelDataName = dataName.replace(/-([a-z])/g, upperCaseLetter);

// Store the data attribute contents into the dataset since
dataset[camelDataName] = dataValue;
}
}

// Prefer the element's `dataset` attribute if it exists
// jQuery 1.x does not correctly handle data attributes with multiple dashes
if ($.fn.jquery && $.fn.jquery.substr(0, 2) == '1.' && $e[0].dataset) {
dataset = $.extend(true, {}, $e[0].dataset, Utils.GetData($e[0]));
} else {
dataset = Utils.GetData($e[0]);
dataset = $.extend(true, {}, $e[0].dataset, dataset);
}

var data = $.extend(true, {}, dataset);
// Prefer our internal data cache if it exists
var data = $.extend(true, {}, Utils.GetData($e[0]), dataset);

data = Utils._convertData(data);

Expand Down
19 changes: 10 additions & 9 deletions src/js/select2/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,9 +277,9 @@ define([

var id = 0;
Utils.GetUniqueElementId = function (element) {
// Get a unique element Id. If element has no id,
// creates a new unique number, stores it in the id
// attribute and returns the new id.
// Get a unique element Id. If element has no id,
// creates a new unique number, stores it in the id
// attribute and returns the new id.
// If an id already exists, it simply returns it.

var select2Id = element.getAttribute('data-select2-id');
Expand All @@ -298,7 +298,7 @@ define([

Utils.StoreData = function (element, name, value) {
// Stores an item in the cache for a specified element.
// name is the cache key.
// name is the cache key.
var id = Utils.GetUniqueElementId(element);
if (!Utils.__cache[id]) {
Utils.__cache[id] = {};
Expand All @@ -309,19 +309,20 @@ define([

Utils.GetData = function (element, name) {
// Retrieves a value from the cache by its key (name)
// name is optional. If no name specified, return
// name is optional. If no name specified, return
// all cache items for the specified element.
// and for a specified element.
var id = Utils.GetUniqueElementId(element);
if (name) {
if (Utils.__cache[id]) {
return Utils.__cache[id][name] != null ?
Utils.__cache[id][name]:
$(element).data(name); // Fallback to HTML5 data attribs.
if (Utils.__cache[id][name] != null) {
return Utils.__cache[id][name];
}
return $(element).data(name); // Fallback to HTML5 data attribs.
}
return $(element).data(name); // Fallback to HTML5 data attribs.
} else {
return Utils.__cache[id];
return Utils.__cache[id];
}
};

Expand Down
File renamed without changes.
21 changes: 21 additions & 0 deletions tests/integration-jq2.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<!doctype html>
<html>
<head>
<link rel="stylesheet" href="vendor/qunit-1.23.1.css" type="text/css" />
<link rel="stylesheet" href="../../dist/css/select2.css" type="text/css" />
</head>
<body>
<div id="qunit"></div>
<div id="qunit-fixture"></div>

<script src="vendor/qunit-1.23.1.js" type="text/javascript"></script>
<script src="vendor/jquery-2.2.4.js" type="text/javascript"></script>
<script src="../dist/js/select2.full.js" type="text/javascript"></script>

<script src="helpers.js" type="text/javascript"></script>

<script src="integration/dom-changes.js" type="text/javascript"></script>
<script src="integration/jquery-calls.js" type="text/javascript"></script>
<script src="integration/select2-methods.js" type="text/javascript"></script>
</body>
</html>
File renamed without changes.
97 changes: 97 additions & 0 deletions tests/unit-jq2.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
<!doctype html>
<html>
<head>
<link rel="stylesheet" href="vendor/qunit-1.23.1.css" type="text/css" />
<link rel="stylesheet" href="../../dist/css/select2.css" type="text/css" />
</head>
<body>
<div id="qunit"></div>
<div id="qunit-fixture">
<div class="event-container">
<select></select>
</div>

<select class="single">
<option>One</option>
</select>

<select class="single-empty"></select>

<select class="single-with-placeholder">
<option>placeholder</option>
<option>One</option>
</select>

<select class="multiple" multiple="multiple">
<option>One</option>
<option>Two</option>
</select>

<select class="groups">
<optgroup label="Test">
<option value="one">One</option>
<option value="two">Two</option>
</optgroup>
<optgroup label="Empty"></optgroup>
</select>

<select class="duplicates">
<option value="one">One</option>
<option value="two">Two</option>
<option value="one">Uno</option>
</select>

<select class="duplicates-multi" multiple="multiple">
<option value="one">One</option>
<option value="two">Two</option>
<option value="one">Uno</option>
</select>

<select class="user-defined"></select>
</div>

<script src="vendor/qunit-1.23.1.js" type="text/javascript"></script>
<script src="vendor/jquery-2.2.4.js" type="text/javascript"></script>
<script src="../dist/js/select2.full.js" type="text/javascript"></script>

<script src="helpers.js" type="text/javascript"></script>

<script src="a11y/selection-tests.js" type="text/javascript"></script>
<script src="a11y/search-tests.js" type="text/javascript"></script>

<script src="data/array-tests.js" type="text/javascript"></script>
<script src="data/base-tests.js" type="text/javascript"></script>
<script src="data/inputData-tests.js" type="text/javascript"></script>
<script src="data/select-tests.js" type="text/javascript"></script>
<script src="data/tags-tests.js" type="text/javascript"></script>
<script src="data/tokenizer-tests.js" type="text/javascript"></script>

<script src="data/maximumInputLength-tests.js" type="text/javascript"></script>
<script src="data/maximumSelectionLength-tests.js" type="text/javascript"></script>
<script src="data/minimumInputLength-tests.js" type="text/javascript"></script>

<script src="dropdown/dropdownCss-tests.js" type="text/javascript"></script>
<script src="dropdown/positioning-tests.js" type="text/javascript"></script>
<script src="dropdown/selectOnClose-tests.js" type="text/javascript"></script>
<script src="dropdown/stopPropagation-tests.js" type="text/javascript"></script>

<script src="options/ajax-tests.js" type="text/javascript"></script>
<script src="options/data-tests.js" type="text/javascript"></script>
<script src="options/deprecated-tests.js" type="text/javascript"></script>
<script src="options/translation-tests.js" type="text/javascript"></script>
<script src="options/width-tests.js" type="text/javascript"></script>

<script src="results/focusing-tests.js" type="text/javascript"></script>

<script src="selection/allowClear-tests.js" type="text/javascript"></script>
<script src="selection/containerCss-tests.js" type="text/javascript"></script>
<script src="selection/multiple-tests.js" type="text/javascript"></script>
<script src="selection/placeholder-tests.js" type="text/javascript"></script>
<script src="selection/search-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="utils/decorator-tests.js" type="text/javascript"></script>
<script src="utils/escapeMarkup-tests.js" type="text/javascript"></script>
</body>
</html>
Loading

0 comments on commit 650035c

Please sign in to comment.