Skip to content

Commit aae0f76

Browse files
author
euromark
committed
Collision free approach to resolve the DOM ID issue in a clean way. Fix to generation of ids for multiple checkboxes. Resolves ticket 4064.
1 parent a57c46f commit aae0f76

File tree

4 files changed

+173
-3
lines changed

4 files changed

+173
-3
lines changed

‎lib/Cake/Test/Case/View/Helper/FormHelperTest.php

Lines changed: 124 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3222,8 +3222,8 @@ public function testSelectAsCheckbox() {
32223222
$expected = array(
32233223
'input' => array('type' => 'hidden', 'name' => 'data[Model][multi_field]', 'value' => '', 'id' => 'ModelMultiField'),
32243224
array('div' => array('class' => 'checkbox')),
3225-
array('input' => array('type' => 'checkbox', 'name' => 'data[Model][multi_field][]', 'value' => '1/2', 'id' => 'ModelMultiField12')),
3226-
array('label' => array('for' => 'ModelMultiField12')),
3225+
array('input' => array('type' => 'checkbox', 'name' => 'data[Model][multi_field][]', 'value' => '1/2', 'id' => 'ModelMultiField1/2')),
3226+
array('label' => array('for' => 'ModelMultiField1/2')),
32273227
'half',
32283228
'/label',
32293229
'/div',
@@ -4150,6 +4150,50 @@ public function testRadioWithCreate() {
41504150
);
41514151
}
41524152

4153+
/**
4154+
* testDomIdSuffix method
4155+
*
4156+
* @return void
4157+
*/
4158+
public function testDomIdSuffix() {
4159+
$result = $this->Form->domIdSuffix('1 string with 1$-dollar signs');
4160+
$this->assertEquals('1StringWith1$-dollarSigns', $result);
4161+
4162+
$result = $this->Form->domIdSuffix('<abc x="foo" y=\'bar\'>');
4163+
$this->assertEquals('AbcX=FooY=Bar', $result);
4164+
4165+
$result = $this->Form->domIdSuffix('1 string with 1$-dollar signs', 'xhtml');
4166+
$this->assertEquals('1StringWith1-dollarSigns', $result);
4167+
4168+
$result = $this->Form->domIdSuffix('<abc x="foo" y=\'bar\'>', 'xhtml');
4169+
$this->assertEquals('AbcXFooYBar', $result);
4170+
}
4171+
4172+
/**
4173+
* testDomIdSuffixCollisionResolvement()
4174+
*
4175+
* @return void
4176+
*/
4177+
public function testDomIdSuffixCollisionResolvement() {
4178+
$result = $this->Form->domIdSuffix('a>b');
4179+
$this->assertEquals('AB', $result);
4180+
4181+
$result = $this->Form->domIdSuffix('a<b');
4182+
$this->assertEquals('AB1', $result);
4183+
4184+
$result = $this->Form->domIdSuffix('a\'b');
4185+
$this->assertEquals('AB2', $result);
4186+
4187+
$result = $this->Form->domIdSuffix('1 string with 1$-dollar', 'xhtml');
4188+
$this->assertEquals('1StringWith1-dollar', $result);
4189+
4190+
$result = $this->Form->domIdSuffix('1 string with 1€-dollar', 'xhtml');
4191+
$this->assertEquals('1StringWith1-dollar1', $result);
4192+
4193+
$result = $this->Form->domIdSuffix('1 string with 1$-dollar', 'xhtml');
4194+
$this->assertEquals('1StringWith1-dollar2', $result);
4195+
}
4196+
41534197
/**
41544198
* testSelect method
41554199
*
@@ -4998,6 +5042,84 @@ public function testSelectMultipleCheckboxes() {
49985042
'/div'
49995043
);
50005044
$this->assertTags($result, $expected);
5045+
5046+
$result = $this->Form->select(
5047+
'Model.multi_field',
5048+
array('a+' => 'first', 'a++' => 'second', 'a+++' => 'third'),
5049+
array('multiple' => 'checkbox')
5050+
);
5051+
$expected = array(
5052+
'input' => array(
5053+
'type' => 'hidden', 'name' => 'data[Model][multi_field]', 'value' => '', 'id' => 'ModelMultiField'
5054+
),
5055+
array('div' => array('class' => 'checkbox')),
5056+
array('input' => array(
5057+
'type' => 'checkbox', 'name' => 'data[Model][multi_field][]',
5058+
'value' => 'a+', 'id' => 'ModelMultiFieldA+'
5059+
)),
5060+
array('label' => array('for' => 'ModelMultiFieldA+')),
5061+
'first',
5062+
'/label',
5063+
'/div',
5064+
array('div' => array('class' => 'checkbox')),
5065+
array('input' => array(
5066+
'type' => 'checkbox', 'name' => 'data[Model][multi_field][]',
5067+
'value' => 'a++', 'id' => 'ModelMultiFieldA++'
5068+
)),
5069+
array('label' => array('for' => 'ModelMultiFieldA++')),
5070+
'second',
5071+
'/label',
5072+
'/div',
5073+
array('div' => array('class' => 'checkbox')),
5074+
array('input' => array(
5075+
'type' => 'checkbox', 'name' => 'data[Model][multi_field][]',
5076+
'value' => 'a+++', 'id' => 'ModelMultiFieldA+++'
5077+
)),
5078+
array('label' => array('for' => 'ModelMultiFieldA+++')),
5079+
'third',
5080+
'/label',
5081+
'/div'
5082+
);
5083+
$this->assertTags($result, $expected);
5084+
5085+
$result = $this->Form->select(
5086+
'Model.multi_field',
5087+
array('a>b' => 'first', 'a<b' => 'second', 'a"b' => 'third'),
5088+
array('multiple' => 'checkbox')
5089+
);
5090+
$expected = array(
5091+
'input' => array(
5092+
'type' => 'hidden', 'name' => 'data[Model][multi_field]', 'value' => '', 'id' => 'ModelMultiField'
5093+
),
5094+
array('div' => array('class' => 'checkbox')),
5095+
array('input' => array(
5096+
'type' => 'checkbox', 'name' => 'data[Model][multi_field][]',
5097+
'value' => 'a&gt;b', 'id' => 'ModelMultiFieldAB2'
5098+
)),
5099+
array('label' => array('for' => 'ModelMultiFieldAB2')),
5100+
'first',
5101+
'/label',
5102+
'/div',
5103+
array('div' => array('class' => 'checkbox')),
5104+
array('input' => array(
5105+
'type' => 'checkbox', 'name' => 'data[Model][multi_field][]',
5106+
'value' => 'a&lt;b', 'id' => 'ModelMultiFieldAB1'
5107+
)),
5108+
array('label' => array('for' => 'ModelMultiFieldAB1')),
5109+
'second',
5110+
'/label',
5111+
'/div',
5112+
array('div' => array('class' => 'checkbox')),
5113+
array('input' => array(
5114+
'type' => 'checkbox', 'name' => 'data[Model][multi_field][]',
5115+
'value' => 'a&quot;b', 'id' => 'ModelMultiFieldAB'
5116+
)),
5117+
array('label' => array('for' => 'ModelMultiFieldAB')),
5118+
'third',
5119+
'/label',
5120+
'/div'
5121+
);
5122+
$this->assertTags($result, $expected);
50015123
}
50025124

50035125
/**

‎lib/Cake/Test/Case/View/HelperTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -851,6 +851,16 @@ public function testClean() {
851851
$this->assertEquals('&amp;lt;script&amp;gt;alert(document.cookie)&amp;lt;/script&amp;gt;', $result);
852852
}
853853

854+
/**
855+
* testDomId method
856+
*
857+
* @return void
858+
*/
859+
public function testDomId() {
860+
$result = $this->Helper->domId('Foo.bar');
861+
$this->assertEquals('FooBar', $result);
862+
}
863+
854864
/**
855865
* testMultiDimensionalField method
856866
*

‎lib/Cake/View/Helper.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
App::uses('Router', 'Routing');
1818
App::uses('Hash', 'Utility');
19+
App::uses('Inflector', 'Utility');
1920

2021
/**
2122
* Abstract base class for all other Helpers in CakePHP.

‎lib/Cake/View/Helper/FormHelper.php

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
App::uses('ClassRegistry', 'Utility');
1818
App::uses('AppHelper', 'View/Helper');
1919
App::uses('Hash', 'Utility');
20+
App::uses('Inflector', 'Utility');
2021

2122
/**
2223
* Form helper library.
@@ -109,6 +110,13 @@ class FormHelper extends AppHelper {
109110
*/
110111
public $validationErrors = array();
111112

113+
/**
114+
* Holds already used DOM ID suffixes to avoid collisions with multiple form field elements.
115+
*
116+
* @var array
117+
*/
118+
protected $_domIdSuffixes = array();
119+
112120
/**
113121
* Copies the validationErrors variable from the View object into this instance
114122
*
@@ -2065,6 +2073,34 @@ public function select($fieldName, $options = array(), $attributes = array()) {
20652073
return implode("\n", $select);
20662074
}
20672075

2076+
/**
2077+
* Generates a valid DOM ID suffix from a string.
2078+
* Also avoids collisions when multiple values are coverted to the same suffix by
2079+
* appending a numeric value.
2080+
*
2081+
* For pre-HTML5 IDs only characters like a-z 0-9 - _ are valid. HTML5 doesn't have that
2082+
* limitation, but to avoid layout issues it still filters out some sensitive chars.
2083+
*
2084+
* @param string $value The value that should be transferred into a DOM ID suffix.
2085+
* @param string $type Doctype to use. Defaults to html5. Anything else will use limited chars.
2086+
* @return string DOM ID
2087+
*/
2088+
public function domIdSuffix($value, $type = 'html5') {
2089+
if ($type === 'html5') {
2090+
$value = str_replace(array('<', '>', ' ', '"', '\''), '_', $value);
2091+
} else {
2092+
$value = preg_replace('~[^\\pL\d-_]+~u', '_', $value);
2093+
}
2094+
$value = Inflector::camelize($value);
2095+
$count = 1;
2096+
$suffix = $value;
2097+
while (in_array($suffix, $this->_domIdSuffixes)) {
2098+
$suffix = $value . $count++;
2099+
}
2100+
$this->_domIdSuffixes[] = $suffix;
2101+
return $suffix;
2102+
}
2103+
20682104
/**
20692105
* Returns a SELECT element for days.
20702106
*
@@ -2609,6 +2645,7 @@ protected function _selectOptions($elements = array(), $parents = array(), $show
26092645
$selectedIsEmpty = ($attributes['value'] === '' || $attributes['value'] === null);
26102646
$selectedIsArray = is_array($attributes['value']);
26112647

2648+
$this->_domIdSuffixes = array();
26122649
foreach ($elements as $name => $title) {
26132650
$htmlOptions = array();
26142651
if (is_array($title) && (!isset($title['name']) || !isset($title['value']))) {
@@ -2677,7 +2714,7 @@ protected function _selectOptions($elements = array(), $parents = array(), $show
26772714
if ($attributes['style'] === 'checkbox') {
26782715
$htmlOptions['value'] = $name;
26792716

2680-
$tagName = $attributes['id'] . Inflector::camelize(Inflector::slug($name));
2717+
$tagName = $attributes['id'] . $this->domIdSuffix($name);
26812718
$htmlOptions['id'] = $tagName;
26822719
$label = array('for' => $tagName);
26832720

0 commit comments

Comments
 (0)