Opened 10 years ago
Last modified 10 years ago
#12895 confirmed Bug
allowedContent for form elements can be improved
Reported by: | jhub | Owned by: | |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | General | Version: | 4.1 |
Keywords: | Cc: |
Description
In the plugin.js for the "Forms" plugin, the code explicitly defines different allowed contents for different form elements:
allowedContent = { checkbox: 'input[type,name,checked]', radio: 'input[type,name,checked]', textfield: 'input[type,name,value,size,maxlength]', textarea: 'textarea[cols,rows,name]', select: 'select[name,size,multiple]; option[value,selected]', button: 'input[type,name,value]', form: 'form[action,name,id,enctype,target,method]', hiddenfield: 'input[type,name,value]', imagebutton: 'input[type,alt,src]{width,height,border,border-width,border-style,margin,float}' },
but since "checkbox", "radio", "textfield", "button", "hiddenfield" and "imagebutton" all define their allowed content for the same tag (with the name <input>), what we end up with is actually that the combination of all the allowed contents for all these form elements is allowed for all of them.
For example, even though "checkbox" explicitly lists that the allowed content is only "input[type,name,checked]", it is still possible to add for example the "src" attribute by hand to an <input> tag with type="checkbox" (in source code mode) and it will not be removed, even though it was initially not listed as allowed content.
I guess it is not removed because the "imagebutton" defines "src" as a valid attribute for the <input> tag, and thus *all* <input> tags now allow the "src" attribute, even tags with type="checkbox" where the "src" attribute does not make any sense.
Proposed fix: For the allowed content, instead of the string notation, use the object notation with a match function, like this:
allowedContent = { checkbox: { input: { match: function (element) { return element.attributes.type == "checkbox"; }, attributes: "type,name,checked" } }, radio: { input: { match: function (element) { return element.attributes.type == "radio"; }, attributes: "type,name,checked" } }, textfield: { input: { match: function (element) { return element.attributes.type == "text" || element.attributes.type == "email" || element.attributes.type == "password" || element.attributes.type == "search" || element.attributes.type == "tel" || element.attributes.type == "url"; }, attributes: "type,name,value,size,maxlength" } }, textarea: 'textarea[cols,rows,name]', select: 'select[name,size,multiple]; option[value,selected]', button: { input: { match: function (element) { return element.attributes.type == "button" || element.attributes.type == "submit" || element.attributes.type == "reset"; }, // TODO attributes: "type,name,value" } }, form: 'form[action,name,id,enctype,target,method]', hiddenfield: { input: { match: function (element) { return element.attributes.type == "hidden"; }, attributes: "type,name,value" } }, imagebutton: { input: { match: function (element) { return element.attributes.type == "image"; }, attributes: "type,alt,src", styles: "width,height,border,border-width,border-style,margin,float" } } }
The proposal above should be equivalent to the original code, only the added "match" functions should make sure that for each type-variant of the <input> tag, only the actually defined allowed content is applied.
Of course you then may want to revise these individual allowed content definitions, for example for the imagebutton, you may want to allow "name" and "value" too?
Change History (3)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
Status: | new → confirmed |
---|---|
Version: | 4.4.5 → 4.3 |
it is still possible to add for example the "src" attribute by hand to an <input> tag with type="checkbox" (in source code mode) and it will not be removed,
It is enough to confirm this issue.
comment:3 Changed 10 years ago by
Version: | 4.3 → 4.1 |
---|
P.S. My proposal of above was only the bare bones code. I should probably have mentioned that you also need to modify the "requiredContent", so that it says
checkbox: 'input[type]'
instead of onlycheckbox: 'input'
(and the same for the 5 other occurences of "input" in the requiredContent). And the six "match" functions in my proposal then all need to includereturn element.attributes.type == "cke-test" || REST_OF_CONDITION"