This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/struts.git
commit 596252789b99f4a7115c2ff05914108ef44748c7 Author: Emil Huseynov <[email protected]> AuthorDate: Mon Nov 2 01:21:45 2026 +0400 WW-5774 Resolve the combobox field by id, not a shared name Review follow-up. The RadioNodeList fix still broke two same-name comboboxes in one form: changing the second select wrote into the first combobox's text field, and a same-named hidden input placed earlier captured the write the same way. Matching by name cannot tell the pair apart. Carry the text field's id on the select (data-struts-combobox-target-id) and resolve it with document.getElementById, which distinguishes same-name pairs and drops the RadioNodeList handling entirely. Add a two-same-name-combobox fixture (ComboBox-6) that pins each select to its own id; fixtures 1-5 updated for the attribute rename. --- .../org/apache/struts2/static/combobox.js | 29 ++++++-------------- .../src/main/resources/template/html5/combobox.ftl | 2 +- .../main/resources/template/simple/combobox.ftl | 2 +- .../apache/struts2/views/jsp/ui/ComboBoxTest.java | 32 +++++++++++++++++++++- .../org/apache/struts2/views/jsp/ui/ComboBox-1.txt | 2 +- .../org/apache/struts2/views/jsp/ui/ComboBox-2.txt | 2 +- .../org/apache/struts2/views/jsp/ui/ComboBox-3.txt | 2 +- .../org/apache/struts2/views/jsp/ui/ComboBox-4.txt | 2 +- .../org/apache/struts2/views/jsp/ui/ComboBox-5.txt | 2 +- .../org/apache/struts2/views/jsp/ui/ComboBox-6.txt | 10 +++++++ 10 files changed, 57 insertions(+), 28 deletions(-) diff --git a/core/src/main/resources/org/apache/struts2/static/combobox.js b/core/src/main/resources/org/apache/struts2/static/combobox.js index e32a53797..de8a6b965 100644 --- a/core/src/main/resources/org/apache/struts2/static/combobox.js +++ b/core/src/main/resources/org/apache/struts2/static/combobox.js @@ -20,11 +20,12 @@ /* * Copies the selected option of a combobox select into its text field. The select carries its settings in data-* * attributes: - * data-struts-combobox-target name of the text field in the same form - * data-struts-header-key value of the header option, which is not copied - * data-struts-empty-option "true" when the empty option is rendered, which is not copied - * The listener is registered once per page, in the capture phase, so it also covers comboboxes rendered later and - * is not stopped by an ancestor's stopPropagation(). + * data-struts-combobox-target-id id of the text field to copy the value into + * data-struts-header-key value of the header option, which is not copied + * data-struts-empty-option "true" when the empty option is rendered, which is not copied + * The field is resolved by id, not by name, so two comboboxes that share a name in one form each drive their own + * text field, and a same-named hidden input cannot capture the write. The listener is registered once per page, in + * the capture phase, so it also covers comboboxes rendered later and is not stopped by an ancestor's stopPropagation(). */ (function () { if (window.strutsComboboxListener) { @@ -37,8 +38,8 @@ if (!select || !select.getAttribute) { return; } - var target = select.getAttribute('data-struts-combobox-target'); - if (target === null || !select.form || select.selectedIndex < 0) { + var targetId = select.getAttribute('data-struts-combobox-target-id'); + if (targetId === null || select.selectedIndex < 0) { return; } var value = select.options[select.selectedIndex].value; @@ -48,19 +49,7 @@ if (select.getAttribute('data-struts-empty-option') === 'true' && value === '') { return; } - var field = select.form.elements[target]; - if (field && typeof field.length === 'number' && field.tagName === undefined) { - // Two controls in the form share the name, so elements[target] is a RadioNodeList rather than the - // text field; assigning value to it would not reach the input. Resolve the first text input in it. - var resolved = null; - for (var i = 0; i < field.length; i++) { - if (field[i].tagName === 'INPUT') { - resolved = field[i]; - break; - } - } - field = resolved; - } + var field = document.getElementById(targetId); if (field) { field.value = value; } diff --git a/core/src/main/resources/template/html5/combobox.ftl b/core/src/main/resources/template/html5/combobox.ftl index 8c3972142..b2d8b64bc 100644 --- a/core/src/main/resources/template/html5/combobox.ftl +++ b/core/src/main/resources/template/html5/combobox.ftl @@ -23,7 +23,7 @@ <#include "/${attributes.templateDir}/${attributes.expandTheme}/controlheader.ftl" /> <#if attributes.list??> <@s.script src="${base}${attributes.staticContentPath}/combobox.js" /> -<select data-struts-combobox="${attributes.escapedId}" data-struts-combobox-target="${attributes.name}"<#rt/> +<select data-struts-combobox="${attributes.escapedId}" data-struts-combobox-target-id="${attributes.id}"<#rt/> <#if attributes.headerKey?? && attributes.headerValue??> data-struts-header-key="${attributes.headerKey}"</#if><#t/> <#if attributes.emptyOption!false> data-struts-empty-option="true"</#if><#t/> <#include "/${attributes.templateDir}/${attributes.expandTheme}/css.ftl" /><#rt/> diff --git a/core/src/main/resources/template/simple/combobox.ftl b/core/src/main/resources/template/simple/combobox.ftl index 01a2fa6ae..beb075f52 100644 --- a/core/src/main/resources/template/simple/combobox.ftl +++ b/core/src/main/resources/template/simple/combobox.ftl @@ -22,7 +22,7 @@ <br /> <#if attributes.list??> <@s.script src="${base}${attributes.staticContentPath}/combobox.js" /> -<select data-struts-combobox="${attributes.escapedId}" data-struts-combobox-target="${attributes.name}"<#rt/> +<select data-struts-combobox="${attributes.escapedId}" data-struts-combobox-target-id="${attributes.id}"<#rt/> <#if attributes.headerKey?? && attributes.headerValue??> data-struts-header-key="${attributes.headerKey}"</#if><#t/> <#if attributes.emptyOption!false> data-struts-empty-option="true"</#if><#t/> <#include "/${attributes.templateDir}/${attributes.expandTheme}/css.ftl" /> diff --git a/core/src/test/java/org/apache/struts2/views/jsp/ui/ComboBoxTest.java b/core/src/test/java/org/apache/struts2/views/jsp/ui/ComboBoxTest.java index 5562d9714..ab752af15 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/ui/ComboBoxTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/ui/ComboBoxTest.java @@ -339,7 +339,7 @@ public class ComboBoxTest extends AbstractUITagTest { public void testHtml5ThemeRendersDataAttributes() throws Exception { // The ComboBox-1..4 fixtures all render the xhtml -> simple path, so a typo in the html5 template's - // data-struts-combobox-target / data-struts-header-key / data-struts-empty-option lines would go + // data-struts-combobox-target-id / data-struts-header-key / data-struts-empty-option lines would go // unnoticed. Render the html5 theme explicitly and pin all three attributes. TestAction testAction = (TestAction) action; testAction.setFoo("banana"); @@ -366,4 +366,34 @@ public class ComboBoxTest extends AbstractUITagTest { verify(ComboBoxTag.class.getResource("ComboBox-5.txt")); } + public void testTwoSameNameCombosResolveByOwnId() throws Exception { + // Two comboboxes sharing a name in one form must each drive their own text field. Resolution keys on the + // field id (data-struts-combobox-target-id), not the shared name, so each select carries its own id and + // combobox.js resolves the right input by getElementById. Matching by name could not tell the pair apart. + TestAction testAction = (TestAction) action; + ArrayList collection = new ArrayList(); + collection.add("foo"); + testAction.setCollection(collection); + + ComboBoxTag first = new ComboBoxTag(); + first.setPageContext(pageContext); + first.setTheme("simple"); + first.setName("tag"); + first.setId("first"); + first.setList("collection"); + first.doStartTag(); + first.doEndTag(); + + ComboBoxTag second = new ComboBoxTag(); + second.setPageContext(pageContext); + second.setTheme("simple"); + second.setName("tag"); + second.setId("second"); + second.setList("collection"); + second.doStartTag(); + second.doEndTag(); + + verify(ComboBoxTag.class.getResource("ComboBox-6.txt")); + } + } diff --git a/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-1.txt b/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-1.txt index ecdc52238..d10167283 100644 --- a/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-1.txt +++ b/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-1.txt @@ -3,7 +3,7 @@ <td class="tdInput"> <input type="text" name="foo" value="hello" id="cb"/><br/> <script src="/static/combobox.js" nonce="r4nd0m"></script> -<select data-struts-combobox="cb" data-struts-combobox-target="foo"> +<select data-struts-combobox="cb" data-struts-combobox-target-id="cb"> <option value="foo">foo</option> <option value="bar">bar</option> <option value="baz">baz</option> diff --git a/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-2.txt b/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-2.txt index 6f98ce713..a7b9bae3c 100644 --- a/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-2.txt +++ b/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-2.txt @@ -3,7 +3,7 @@ <td class="tdInput"> <input type="text" name="myFavouriteFruit" value="banana" id="myFavouriteFruit"/><br/> <script src="/static/combobox.js"></script> -<select data-struts-combobox="myFavouriteFruit" data-struts-combobox-target="myFavouriteFruit" data-struts-header-key="-1" data-struts-empty-option="true"> +<select data-struts-combobox="myFavouriteFruit" data-struts-combobox-target-id="myFavouriteFruit" data-struts-header-key="-1" data-struts-empty-option="true"> <option value="-1">--- Please Select ---</option> <option value=""></option> <option value="apple">apple</option> diff --git a/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-3.txt b/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-3.txt index 6f98ce713..a7b9bae3c 100644 --- a/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-3.txt +++ b/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-3.txt @@ -3,7 +3,7 @@ <td class="tdInput"> <input type="text" name="myFavouriteFruit" value="banana" id="myFavouriteFruit"/><br/> <script src="/static/combobox.js"></script> -<select data-struts-combobox="myFavouriteFruit" data-struts-combobox-target="myFavouriteFruit" data-struts-header-key="-1" data-struts-empty-option="true"> +<select data-struts-combobox="myFavouriteFruit" data-struts-combobox-target-id="myFavouriteFruit" data-struts-header-key="-1" data-struts-empty-option="true"> <option value="-1">--- Please Select ---</option> <option value=""></option> <option value="apple">apple</option> diff --git a/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-4.txt b/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-4.txt index 23f0e0caa..25c87b470 100644 --- a/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-4.txt +++ b/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-4.txt @@ -3,7 +3,7 @@ <td class="tdInput"> <input type="text" name="foo" value="hello" id="cb['"."']=bc(){};//"/><br/> <script src="/static/combobox.js"></script> - <select data-struts-combobox="cb__________bc_______" data-struts-combobox-target="foo"> + <select data-struts-combobox="cb__________bc_______" data-struts-combobox-target-id="cb['"."']=bc(){};//"> <option value="foo">foo</option> </select> </td> diff --git a/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-5.txt b/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-5.txt index ab9b56fba..d24256ca3 100644 --- a/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-5.txt +++ b/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-5.txt @@ -1,6 +1,6 @@ <input type="text" name="myFavouriteFruit" value="banana" id="myFavouriteFruit"/> <script src="/static/combobox.js"></script> -<select data-struts-combobox="myFavouriteFruit" data-struts-combobox-target="myFavouriteFruit" data-struts-header-key="-1" data-struts-empty-option="true"> +<select data-struts-combobox="myFavouriteFruit" data-struts-combobox-target-id="myFavouriteFruit" data-struts-header-key="-1" data-struts-empty-option="true"> <option value="-1">--- Please Select ---</option> <option value=""></option> <option value="apple">apple</option> diff --git a/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-6.txt b/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-6.txt new file mode 100644 index 000000000..060ad9d72 --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-6.txt @@ -0,0 +1,10 @@ +<input type="text" name="tag" value="" id="first"/><br/> +<script src="/static/combobox.js"></script> +<select data-struts-combobox="first" data-struts-combobox-target-id="first"> + <option value="foo">foo</option> +</select> +<input type="text" name="tag" value="" id="second"/><br/> +<script src="/static/combobox.js"></script> +<select data-struts-combobox="second" data-struts-combobox-target-id="second"> + <option value="foo">foo</option> +</select>
