Skip to content

Commit efbfd14

Browse files
kevin-brownkevin-brown
authored
Remove selection title attribute if text is empty (select2#5589)
This fixes a bug that was introduced in Select2 4.0.0 and only partially fixed in Select2 4.0.6-rc.0 where the `title` attribute that is set on the selection container (or individual selections, for a multiple select) is not cleared when the text/title of the option is not set. In most cases, users no longer see this issue because the `text` property of most data objects is set, so the `title` attribute will always be cleared correctly. There was a bug for cases where the `text` property was not set, or where the `text` property was set to an empty string, that resulted in the `title` attribute persisting with the incorrect value. We have fixed this issue by always removing the `title` attribute from the selection (or not adding it in the first place, for a multiple select) when the `text` and `title` properties of the data object are empty or not present. This also adds in a series of tests to ensure the `title` attribute is set properly in a variety of cases, building upon the ones that already existed. Fixes select2#3895
1 parent 6645ffd commit efbfd14

4 files changed

Lines changed: 236 additions & 9 deletions

File tree

‎src/js/select2/selection/multiple.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,12 @@ define([
9595
var formatted = this.display(selection, $selection);
9696

9797
$selection.append(formatted);
98-
$selection.attr('title', selection.title || selection.text);
98+
99+
var title = selection.title || selection.text;
100+
101+
if (title) {
102+
$selection.attr('title', title);
103+
}
99104

100105
Utils.StoreData($selection[0], 'data', selection);
101106

‎src/js/select2/selection/single.js‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,14 @@ define([
9393
var formatted = this.display(selection, $rendered);
9494

9595
$rendered.empty().append(formatted);
96-
$rendered.attr('title', selection.title || selection.text);
96+
97+
var title = selection.title || selection.text;
98+
99+
if (title) {
100+
$rendered.attr('title', title);
101+
} else {
102+
$rendered.removeAttr('title');
103+
}
97104
};
98105

99106
return SingleSelection;

‎tests/selection/multiple-tests.js‎

Lines changed: 110 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,119 @@ test('empty update clears the selection', function (assert) {
7272
var $rendered = $selection.find('.select2-selection__rendered');
7373

7474
$rendered.text('testing');
75-
$rendered.attr('title', 'testing');
7675

7776
selection.update([]);
7877

79-
assert.equal($rendered.text(), '');
80-
assert.equal($rendered.attr('title'), undefined);
78+
assert.equal(
79+
$rendered.text(),
80+
'',
81+
'There should have been nothing rendered'
82+
);
83+
});
84+
85+
test('empty update clears the selection title', function (assert) {
86+
var selection = new MultipleSelection(
87+
$('#qunit-fixture .multiple'),
88+
options
89+
);
90+
91+
var $selection = selection.render();
92+
93+
selection.update([]);
94+
95+
var $rendered = $selection.find('.select2-selection__rendered li');
96+
97+
assert.equal(
98+
$rendered.attr('title'),
99+
undefined,
100+
'The title should be removed if nothing is rendered'
101+
);
102+
});
103+
104+
test('update sets the title to the data text', function (assert) {
105+
var selection = new MultipleSelection(
106+
$('#qunit-fixture .multiple'),
107+
options
108+
);
109+
110+
var $selection = selection.render();
111+
112+
selection.update([{
113+
text: 'test'
114+
}]);
115+
116+
var $rendered = $selection.find('.select2-selection__rendered li');
117+
118+
assert.equal(
119+
$rendered.attr('title'),
120+
'test',
121+
'The title should have been set to the text'
122+
);
123+
});
124+
125+
test('update sets the title to the data title', function (assert) {
126+
var selection = new MultipleSelection(
127+
$('#qunit-fixture .multiple'),
128+
options
129+
);
130+
131+
var $selection = selection.render();
132+
133+
selection.update([{
134+
text: 'test',
135+
title: 'correct'
136+
}]);
137+
138+
var $rendered = $selection.find('.select2-selection__rendered li');
139+
140+
assert.equal(
141+
$rendered.attr('title'),
142+
'correct',
143+
'The title should have taken precedence over the text'
144+
);
145+
});
146+
147+
test('update should clear title for placeholder options', function (assert) {
148+
var selection = new MultipleSelection(
149+
$('#qunit-fixture .multiple'),
150+
options
151+
);
152+
153+
var $selection = selection.render();
154+
155+
selection.update([{
156+
id: '',
157+
text: ''
158+
}]);
159+
160+
var $rendered = $selection.find('.select2-selection__rendered li');
161+
162+
assert.equal(
163+
$rendered.attr('title'),
164+
undefined,
165+
'The title should be removed if a placeholder is rendered'
166+
);
167+
});
168+
169+
test('update should clear title for options without text', function (assert) {
170+
var selection = new MultipleSelection(
171+
$('#qunit-fixture .multiple'),
172+
options
173+
);
174+
175+
var $selection = selection.render();
176+
177+
selection.update([{
178+
id: ''
179+
}]);
180+
181+
var $rendered = $selection.find('.select2-selection__rendered li');
182+
183+
assert.equal(
184+
$rendered.attr('title'),
185+
undefined,
186+
'The title should be removed if there is no text or title property'
187+
);
81188
});
82189

83190
test('escapeMarkup is being used', function (assert) {

‎tests/selection/single-tests.js‎

Lines changed: 112 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,15 @@ test('templateSelection can addClass', function (assert) {
5050
);
5151

5252
var $container = selection.selectionContainer();
53-
53+
5454
var out = selection.display({
5555
text: 'test'
5656
}, $container);
5757

5858
assert.ok(called);
5959

6060
assert.equal(out, 'test');
61-
61+
6262
assert.ok($container.hasClass('testclass'));
6363
});
6464

@@ -72,12 +72,34 @@ test('empty update clears the selection', function (assert) {
7272
var $rendered = $selection.find('.select2-selection__rendered');
7373

7474
$rendered.text('testing');
75+
76+
selection.update([]);
77+
78+
assert.equal(
79+
$rendered.text(),
80+
'',
81+
'There should have been nothing rendered'
82+
);
83+
});
84+
85+
test('empty update clears the selection title', function (assert) {
86+
var selection = new SingleSelection(
87+
$('#qunit-fixture .single'),
88+
options
89+
);
90+
91+
var $selection = selection.render();
92+
var $rendered = $selection.find('.select2-selection__rendered');
93+
7594
$rendered.attr('title', 'testing');
7695

7796
selection.update([]);
7897

79-
assert.equal($rendered.text(), '');
80-
assert.equal($rendered.attr('title'), undefined);
98+
assert.equal(
99+
$rendered.attr('title'),
100+
undefined,
101+
'The title should be removed if nothing is rendered'
102+
);
81103
});
82104

83105
test('update renders the data text', function (assert) {
@@ -96,6 +118,92 @@ test('update renders the data text', function (assert) {
96118
assert.equal($rendered.text(), 'test');
97119
});
98120

121+
test('update sets the title to the data text', function (assert) {
122+
var selection = new SingleSelection(
123+
$('#qunit-fixture .single'),
124+
options
125+
);
126+
127+
var $selection = selection.render();
128+
var $rendered = $selection.find('.select2-selection__rendered');
129+
130+
selection.update([{
131+
text: 'test'
132+
}]);
133+
134+
assert.equal(
135+
$rendered.attr('title'),
136+
'test',
137+
'The title should have been set to the text'
138+
);
139+
});
140+
141+
test('update sets the title to the data title', function (assert) {
142+
var selection = new SingleSelection(
143+
$('#qunit-fixture .single'),
144+
options
145+
);
146+
147+
var $selection = selection.render();
148+
var $rendered = $selection.find('.select2-selection__rendered');
149+
150+
selection.update([{
151+
text: 'test',
152+
title: 'correct'
153+
}]);
154+
155+
assert.equal(
156+
$rendered.attr('title'),
157+
'correct',
158+
'The title should have taken precedence over the text'
159+
);
160+
});
161+
162+
test('update should clear title for placeholder options', function (assert) {
163+
var selection = new SingleSelection(
164+
$('#qunit-fixture .single'),
165+
options
166+
);
167+
168+
var $selection = selection.render();
169+
var $rendered = $selection.find('.select2-selection__rendered');
170+
171+
$rendered.attr('title', 'testing');
172+
173+
selection.update([{
174+
id: '',
175+
text: ''
176+
}]);
177+
178+
assert.equal(
179+
$rendered.attr('title'),
180+
undefined,
181+
'The title should be removed if a placeholder is rendered'
182+
);
183+
});
184+
185+
test('update should clear title for options without text', function (assert) {
186+
var selection = new SingleSelection(
187+
$('#qunit-fixture .single'),
188+
options
189+
);
190+
191+
var $selection = selection.render();
192+
var $rendered = $selection.find('.select2-selection__rendered');
193+
194+
$rendered.attr('title', 'testing');
195+
196+
selection.update([{
197+
id: ''
198+
}]);
199+
200+
assert.equal(
201+
$rendered.attr('title'),
202+
undefined,
203+
'The title should be removed if there is no text or title property'
204+
);
205+
});
206+
99207
test('escapeMarkup is being used', function (assert) {
100208
var selection = new SingleSelection(
101209
$('#qunit-fixture .single'),

0 commit comments

Comments
 (0)