Skip to content

WW-5772 Remove inline event handlers from optiontransferselect, inputtransferselect, updownselect and combobox #2012

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lukaszlenart merged 2 commits into from
Oct 15, 2026
Merged

WW-5772 Remove inline event handlers from optiontransferselect, inputtransferselect, updownselect and combobox #2012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
243984f
WW-5772 Bind transfer-select buttons from data attributes instead of …
aeonframework Oct 14, 2026
c01fc86
WW-5772 Bind combobox select change handler from a nonce'd script
aeonframework Oct 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions core/src/main/resources/org/apache/struts2/static/inputtransferselect.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,46 @@ function swapOptions(objTargetElement, first, second) {
opt[first] = temp2;
opt[second] = temp;
}

/*
* Buttons rendered by the inputtransferselect tag carry their action and target select id in data-* attributes
* instead of inline onclick handlers, so the tag works under a Content-Security-Policy without 'unsafe-inline':
* data-struts-inputtransferselect="add|remove|removeAll|moveUp|moveDown", data-struts-target, optional
* data-struts-header-key
* The listener is registered once per page, in the capture phase, so it is not stopped by an ancestor's
* stopPropagation().
*/
(function () {
if (window.strutsInputTransferSelectListener) {
return;
}
window.strutsInputTransferSelectListener = true;

document.addEventListener('click', function (event) {
var button = event.target;
if (!button || !button.getAttribute) {
return;
}
var action = button.getAttribute('data-struts-inputtransferselect');
if (!action) {
return;
}
var targetId = button.getAttribute('data-struts-target');
var target = document.getElementById(targetId);
if (!target) {
return;
}
var headerKey = button.getAttribute('data-struts-header-key') || '';
if (action === 'add') {
addOption(document.getElementById(targetId + '_input'), target);
} else if (action === 'remove') {
removeOptions(target);
} else if (action === 'removeAll') {
removeAllOptions(target);
} else if (action === 'moveUp') {
moveOptionUp(target, 'key', headerKey);
} else if (action === 'moveDown') {
moveOptionDown(target, 'key', headerKey);
}
}, true);
})();
93 changes: 93 additions & 0 deletions core/src/main/resources/org/apache/struts2/static/optiontransferselect.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,3 +189,96 @@ function swapOptions(objTargetElement, first, second) {
opt[first] = temp2;
opt[second] = temp;
}

/*
* Buttons rendered by the optiontransferselect and updownselect tags carry their action, target select ids and header
* keys in data-* attributes instead of inline onclick handlers, so the tags work under a Content-Security-Policy
* without 'unsafe-inline':
* data-struts-updownselect="moveUp|moveDown|selectAll", data-struts-target, optional data-struts-header-key
* data-struts-optiontransferselect="leftUp|leftDown|rightUp|rightDown|addToLeft|addToRight|addAllToLeft|
* addAllToRight|selectAll", data-struts-left, data-struts-right, optional data-struts-header-key and
* data-struts-double-header-key
* The listener is registered once per page, in the capture phase, so the framework action still runs before a
* developer-supplied onclick attribute on the same button and is not stopped by an ancestor's stopPropagation().
*/
(function () {
if (window.strutsOptionTransferSelectListener) {
return;
}
window.strutsOptionTransferSelectListener = true;

function headerKey(button, name) {
return button.hasAttribute(name) ? button.getAttribute(name) : undefined;

@lukaszlenart lukaszlenart Oct 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not introduced here, but worth knowing since the ticket says a header key with a quote now works: the key still goes through compile(), which builds new RegExp("^" + ptn + "$") without escaping. A key like [none makes every button throw Invalid regular expression, and (none) doesn't match itself, so the header option gets moved/selected with the rest. The old inline handlers had the same behaviour, so no change needed in this PR.

}

function selectAll(target, key) {
if (key === undefined) {
selectAllOptions(target);
} else {
selectAllOptionsExceptSome(target, 'key', key);
}
}

function transfer(move, source, target, key) {
if (key === undefined) {
move(source, target, false, '');
} else {
move(source, target, false, key, '');
}
}

function updownselect(button, action) {
var target = document.getElementById(button.getAttribute('data-struts-target'));
var key = headerKey(button, 'data-struts-header-key');
if (action === 'moveUp') {
moveOptionUp(target, 'key', key === undefined ? '' : key);
} else if (action === 'moveDown') {
moveOptionDown(target, 'key', key === undefined ? '' : key);
} else if (action === 'selectAll') {
selectAll(target, key);
}
}

function optiontransferselect(button, action) {
var left = document.getElementById(button.getAttribute('data-struts-left'));
var right = document.getElementById(button.getAttribute('data-struts-right'));
var leftKey = headerKey(button, 'data-struts-header-key');
var rightKey = headerKey(button, 'data-struts-double-header-key');
if (action === 'leftDown') {
moveOptionDown(left, 'key', leftKey === undefined ? '' : leftKey);
} else if (action === 'leftUp') {
moveOptionUp(left, 'key', leftKey === undefined ? '' : leftKey);
} else if (action === 'rightDown') {
moveOptionDown(right, 'key', rightKey === undefined ? '' : rightKey);
} else if (action === 'rightUp') {
moveOptionUp(right, 'key', rightKey === undefined ? '' : rightKey);
} else if (action === 'addToLeft') {
transfer(moveSelectedOptions, right, left, rightKey);
} else if (action === 'addToRight') {
transfer(moveSelectedOptions, left, right, leftKey);
} else if (action === 'addAllToLeft') {
transfer(moveAllOptions, right, left, rightKey);
} else if (action === 'addAllToRight') {
transfer(moveAllOptions, left, right, leftKey);
} else if (action === 'selectAll') {
selectAll(left, leftKey);
selectAll(right, rightKey);
}
}

document.addEventListener('click', function (event) {
var button = event.target;
if (!button || !button.getAttribute) {
return;
}
var action = button.getAttribute('data-struts-updownselect');
if (action) {
updownselect(button, action);
return;
}
action = button.getAttribute('data-struts-optiontransferselect');
if (action) {
optiontransferselect(button, action);
}
}, true);
})();
15 changes: 14 additions & 1 deletion core/src/main/resources/template/html5/combobox.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ function autoPopulate_${attributes.escapedId}(targetElement) {

<#include "/${attributes.templateDir}/${attributes.expandTheme}/controlheader.ftl" />
<#if attributes.list??>
<select onChange="autoPopulate_${attributes.escapedId}(this);"<#rt/>
<select data-struts-combobox="${attributes.escapedId}"<#rt/>
<#include "/${attributes.templateDir}/${attributes.expandTheme}/css.ftl" /><#rt/>
<#if attributes.disabled!false>
disabled="disabled"<#rt/>
Expand Down Expand Up @@ -97,5 +97,18 @@ function autoPopulate_${attributes.escapedId}(targetElement) {
>${tmpListValue}</option><#rt/>
</@s.iterator>
</select><#rt/>
<@s.script>
(function () {
var selects = document.querySelectorAll('select[data-struts-combobox="${attributes.escapedId}"]');
for (var i = 0; i < selects.length; i++) {
if (!selects[i].strutsComboboxBound) {
selects[i].strutsComboboxBound = true;
selects[i].addEventListener('change', function () {
autoPopulate_${attributes.escapedId}(this);
});
}
}
})();
</@s.script>
</#if>
<#include "/${attributes.templateDir}/${attributes.expandTheme}/controlfooter.ftl" /><#rt/>
15 changes: 14 additions & 1 deletion core/src/main/resources/template/simple/combobox.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
<#include "/${attributes.templateDir}/simple/text.ftl" />
<br />
<#if attributes.list??>
<select onChange="autoPopulate_${attributes.escapedId}(this);"<#rt/>
<select data-struts-combobox="${attributes.escapedId}"<#rt/>
<#include "/${attributes.templateDir}/${attributes.expandTheme}/css.ftl" />
<#if attributes.disabled!false>
disabled="disabled"<#rt/>
Expand Down Expand Up @@ -98,4 +98,17 @@
</option><#lt/>
</@s.iterator>
</select>
<@s.script>

@lukaszlenart lukaszlenart Oct 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Combobox takes a different route from the transfer selects: a per-instance inline script (with the nonce) rather than a static delegated listener. That works under a nonce-based CSP, but:

  • a hash-based or static-scripts-only policy still blocks it,
  • this block is duplicated verbatim in html5/combobox.ftl,
  • N comboboxes in an iterator emit N blocks, each running querySelectorAll over the page.

Fine to land as is; a static combobox.js with one delegated change listener is tracked in WW-5774.

(function () {
var selects = document.querySelectorAll('select[data-struts-combobox="${attributes.escapedId}"]');
for (var i = 0; i < selects.length; i++) {
if (!selects[i].strutsComboboxBound) {
selects[i].strutsComboboxBound = true;
selects[i].addEventListener('change', function () {
autoPopulate_${attributes.escapedId}(this);
});
}
}
})();
</@s.script>
</#if>
16 changes: 7 additions & 9 deletions core/src/main/resources/template/simple/inputtransferselect.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
<#if attributes.buttonCssStyle?has_content>
style="${attributes.buttonCssStyle}"
</#if><#t/>
value="${addLabel}" onclick="addOption(document.getElementById('${attributes.id}_input'), document.getElementById('${attributes.id}'))" /><br /><br />
value="${addLabel}" data-struts-inputtransferselect="add" data-struts-target="${attributes.id}" /><br /><br />
<#t/>
<#assign removeLabel=(attributes.removeLabel!"<-") /><#t/>
<input type="button"
Expand All @@ -79,7 +79,7 @@
<#if attributes.buttonCssStyle?has_content>
style="${attributes.buttonCssStyle}"
</#if><#t/>
value="${removeLabel}" onclick="removeOptions(document.getElementById('${attributes.id}'))" /><br /><br />
value="${removeLabel}" data-struts-inputtransferselect="remove" data-struts-target="${attributes.id}" /><br /><br />
<#t/>
<#assign removeAllLabel=(attributes.removeAllLabel!"<<--") /><#t/>
<input type="button"
Expand All @@ -89,7 +89,7 @@
<#if attributes.buttonCssStyle?has_content>
style="${attributes.buttonCssStyle}"
</#if><#t/>
value="${removeAllLabel}" onclick="removeAllOptions(document.getElementById('${attributes.id}'))" /><br /><br />
value="${removeAllLabel}" data-struts-inputtransferselect="removeAll" data-struts-target="${attributes.id}" /><br /><br />
</td>
<td>
<#if attributes.rightTitle?has_content><#t/>
Expand All @@ -99,20 +99,18 @@
<#include "/${attributes.templateDir}/simple/select.ftl" />
<#if attributes.allowUpDown!true>
<input type="button"
data-struts-inputtransferselect="moveDown" data-struts-target="${attributes.id}"
<#if attributes.headerKey?has_content>
onclick="moveOptionDown(document.getElementById('${attributes.id}'), 'key', '${attributes.headerKey}');"
<#else>
onclick="moveOptionDown(document.getElementById('${attributes.id}'), 'key', '');"
data-struts-header-key="${attributes.headerKey}"
</#if>
<#if attributes.downLabel?has_content>
value="${attributes.downLabel}"
</#if>
/>
<input type="button"
data-struts-inputtransferselect="moveUp" data-struts-target="${attributes.id}"
<#if attributes.headerKey?has_content>
onclick="moveOptionUp(document.getElementById('${attributes.id}'), 'key', '${attributes.headerKey}');"
<#else>
onclick="moveOptionUp(document.getElementById('${attributes.id}'), 'key', '');"
data-struts-header-key="${attributes.headerKey}"
</#if>
<#if attributes.upLabel?has_content>
value="${attributes.upLabel}"
Expand Down
Loading
Loading