This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5847-doubleselect-multiple-preselect in repository https://gitbox.apache.org/repos/asf/struts.git
commit 1fb91005060c52965c186477893fe77c7a3babdd Author: Lukasz Lenart <[email protected]> AuthorDate: Sat Nov 7 20:28:38 2026 +0100 WW-5847 fix(core): preselect multiple values in the doubleselect second list With doubleMultiple set, the simple theme interpolated the bound value of the second list into the generated script unquoted, so a plain String became a bare identifier, a Collection rendered through toString, and a String[] failed the template. The branch was also selected by the attribute's presence, so doubleMultiple="false" took it. DoubleListUIBean now also exposes the value as a list of strings, and the template emits it as an array of quoted, JS-escaped string literals, the same way the single-value branch and the option values are already written. The branch is selected by the attribute's boolean value. Co-Authored-By: Claude Mythos 5.1 <[email protected]> --- .../struts2/components/DoubleListUIBean.java | 46 ++++--- .../resources/template/simple/doubleselect.ftl | 9 +- .../views/jsp/ui/DoubleSelectMultipleTest.java | 136 +++++++++++++++++++++ .../apache/struts2/views/jsp/ui/DoubleSelect-7.txt | 50 ++++++++ 4 files changed, 223 insertions(+), 18 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/components/DoubleListUIBean.java b/core/src/main/java/org/apache/struts2/components/DoubleListUIBean.java index 56a1de8a7..d0c174684 100644 --- a/core/src/main/java/org/apache/struts2/components/DoubleListUIBean.java +++ b/core/src/main/java/org/apache/struts2/components/DoubleListUIBean.java @@ -18,11 +18,15 @@ */ package org.apache.struts2.components; +import org.apache.struts2.util.MakeIterator; import org.apache.struts2.util.ValueStack; import org.apache.struts2.views.annotations.StrutsTagAttribute; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; import java.util.Map; /** @@ -168,21 +172,9 @@ public abstract class DoubleListUIBean extends ListUIBean { } } - Class<?> valueClazz = getValueClassType(); - - if (valueClazz != null) { - if (doubleValue != null) { - addParameter("doubleNameValue", findValue(doubleValue, valueClazz)); - } else if (doubleName != null) { - addParameter("doubleNameValue", findValue(doubleName, valueClazz)); - } - } else { - if (doubleValue != null) { - addParameter("doubleNameValue", findValue(doubleValue)); - } else if (doubleName != null) { - addParameter("doubleNameValue", findValue(doubleName)); - } - } + Object doubleNameValue = findDoubleNameValue(); + addParameter("doubleNameValue", doubleNameValue); + addParameter("doubleNameValues", toStrings(doubleNameValue)); Form form = (Form) findAncestor(Form.class); if (doubleId != null) { @@ -267,6 +259,30 @@ public abstract class DoubleListUIBean extends ListUIBean { } } + private Object findDoubleNameValue() { + String expression = doubleValue != null ? doubleValue : doubleName; + if (expression == null) { + return null; + } + Class<?> valueClazz = getValueClassType(); + return valueClazz != null ? findValue(expression, valueClazz) : findValue(expression); + } + + private static List<String> toStrings(Object value) { + List<String> strings = new ArrayList<>(); + Iterator<?> items = MakeIterator.convert(value); + if (items == null) { + return strings; + } + while (items.hasNext()) { + Object item = items.next(); + if (item != null) { + strings.add(item.toString()); + } + } + return strings; + } + @StrutsTagAttribute(description = "The second iterable source to populate from.", required = true) public void setDoubleList(String doubleList) { this.doubleList = doubleList; diff --git a/core/src/main/resources/template/simple/doubleselect.ftl b/core/src/main/resources/template/simple/doubleselect.ftl index 342f199c2..3a999e54b 100644 --- a/core/src/main/resources/template/simple/doubleselect.ftl +++ b/core/src/main/resources/template/simple/doubleselect.ftl @@ -191,6 +191,9 @@ </@s.iterator> var ${attributes.escapedId}Temp = document.${attributes.formName}.${attributes.doubleId}; + <#if attributes.doubleNameValue?? && attributes.doubleMultiple!false> + var ${attributes.escapedId}Selected = [<#list attributes.doubleNameValues as selectedValue>"${selectedValue?js_string}"<#sep>, </#list>]; + </#if> <#assign itemCount = startCount/> <#assign redirectTo = 0/> <@s.iterator value="attributes.list"> @@ -214,9 +217,9 @@ for (var i = 0; i < ${attributes.escapedId}Group[x].length; i++) { ${attributes.escapedId}Temp.options[i] = new Option(${attributes.escapedId}Group[x][i].text, ${attributes.escapedId}Group[x][i].value); <#if attributes.doubleNameValue??> - <#if attributes.doubleMultiple??> - for (var j = 0; j < ${attributes.doubleNameValue}.length; j++) { - if (${attributes.escapedId}Temp.options[i].value == ${attributes.doubleNameValue?js_string}[j]) { + <#if attributes.doubleMultiple!false> + for (var j = 0; j < ${attributes.escapedId}Selected.length; j++) { + if (${attributes.escapedId}Temp.options[i].value == ${attributes.escapedId}Selected[j]) { ${attributes.escapedId}Temp.options[i].selected = true; selected = true; } diff --git a/core/src/test/java/org/apache/struts2/views/jsp/ui/DoubleSelectMultipleTest.java b/core/src/test/java/org/apache/struts2/views/jsp/ui/DoubleSelectMultipleTest.java new file mode 100644 index 000000000..47250c661 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/views/jsp/ui/DoubleSelectMultipleTest.java @@ -0,0 +1,136 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.views.jsp.ui; + +import org.apache.struts2.TestAction; +import org.apache.struts2.views.jsp.AbstractUITagTest; +import org.apache.struts2.views.jsp.ui.DoubleSelectTest.Country; +import org.apache.struts2.views.jsp.ui.DoubleSelectTest.Region; + +import java.util.ArrayList; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +public class DoubleSelectMultipleTest extends AbstractUITagTest { + + public static class SelectedRegions { + private final String[] selectedRegions; + + public SelectedRegions(String... selectedRegions) { + this.selectedRegions = selectedRegions; + } + + public String[] getSelectedRegions() { + return selectedRegions; + } + } + + private DoubleSelectTag tag; + + @Override + protected void setUp() throws Exception { + super.setUp(); + + List<Region> belgiumRegions = new ArrayList<>(); + belgiumRegions.add(new Region("Antwerp", "AN")); + belgiumRegions.add(new Region("Gent", "GN")); + belgiumRegions.add(new Region("Brugge", "BRG")); + List<Region> franceRegions = new ArrayList<>(); + franceRegions.add(new Region("Paris", "PA")); + franceRegions.add(new Region("Bordeaux", "BOR")); + List<Country> countries = new ArrayList<>(); + countries.add(new Country("Belgium", "BE", belgiumRegions)); + countries.add(new Country("France", "FR", franceRegions)); + ((TestAction) action).setList2(countries); + + tag = new DoubleSelectTag(); + tag.setPageContext(pageContext); + tag.setLabel("mylabel"); + tag.setName("foo"); + tag.setValue("'BE'"); + tag.setDoubleName("region"); + tag.setList("list2"); + tag.setDoubleList("regions"); + tag.setListKey("iso"); + tag.setDoubleListKey("key"); + tag.setListValue("name"); + tag.setDoubleListValue("name"); + tag.setFormName("inputForm"); + } + + private String render() throws Exception { + tag.doStartTag(); + tag.doEndTag(); + return writer.toString(); + } + + public void testDoubleMultipleWithCollectionValue() throws Exception { + tag.setDoubleMultiple("true"); + tag.setDoubleValue("{'AN', 'GN'}"); + + render(); + + verify(SelectTag.class.getResource("DoubleSelect-7.txt")); + } + + public void testDoubleMultipleWithStringValue() throws Exception { + tag.setDoubleMultiple("true"); + tag.setDoubleValue("'AN'"); + + assertThat(render()).contains("var fooSelected = [\"AN\"];"); + } + + public void testDoubleMultipleWithArrayValue() throws Exception { + stack.push(new SelectedRegions("AN", "BRG")); + tag.setDoubleMultiple("true"); + tag.setDoubleValue("selectedRegions"); + + assertThat(render()).contains("var fooSelected = [\"AN\", \"BRG\"];"); + } + + public void testDoubleMultipleValuesRenderAsStringLiterals() throws Exception { + stack.push(new SelectedRegions("a'b\\c", "d\"e")); + tag.setDoubleMultiple("true"); + tag.setDoubleValue("selectedRegions"); + + assertThat(render()).contains("var fooSelected = [\"a\\'b\\\\c\", \"d\\"e\"];"); + } + + public void testDoubleMultipleValueCannotCloseTheScriptBlock() throws Exception { + stack.push(new SelectedRegions("</script><b>")); + tag.setDoubleMultiple("true"); + tag.setDoubleValue("selectedRegions"); + + String output = render(); + + assertThat(output).contains("var fooSelected = [\"<\\/script><b>\"];"); + assertThat(output.indexOf("</script>")).isEqualTo(output.lastIndexOf("</script>")); + } + + public void testDoubleMultipleFalseUsesTheSingleValuePath() throws Exception { + tag.setDoubleMultiple("false"); + tag.setDoubleValue("'AN'"); + + String output = render(); + + assertThat(output).contains("fooTemp.options[i].value == 'AN'"); + assertThat(output).doesNotContain("fooSelected"); + } +} diff --git a/core/src/test/resources/org/apache/struts2/views/jsp/ui/DoubleSelect-7.txt b/core/src/test/resources/org/apache/struts2/views/jsp/ui/DoubleSelect-7.txt new file mode 100644 index 000000000..0f85f8e83 --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/views/jsp/ui/DoubleSelect-7.txt @@ -0,0 +1,50 @@ +<tr> + <td class="tdLabel"><label for="foo" class="label">mylabel:</label></td> + <td class="tdInput"> +<select name="foo" id="foo" onchange="fooRedirect(this.selectedIndex)"> + <option value="BE" selected="selected">Belgium</option> + <option value="FR">France</option> +</select> +<br/> +<select name="region" id="region" multiple="multiple"> +</select> +<input type="hidden" id="__multiselect_region" name="__multiselect_region" value=""/> +<script> + var fooGroup = new Array(2 + 0); + for (var i = 0; i < (2 + 0); i++) { + fooGroup[i] = []; + } + + fooGroup[0][0] = new Option("Antwerp", "AN"); + fooGroup[0][1] = new Option("Gent", "GN"); + fooGroup[0][2] = new Option("Brugge", "BRG"); + fooGroup[1][0] = new Option("Paris", "PA"); + fooGroup[1][1] = new Option("Bordeaux", "BOR"); + + var fooTemp = document.inputForm.region; + var fooSelected = ["AN", "GN"]; + fooRedirect(0); + + function fooRedirect(x) { + var selected = false; + for (var m = fooTemp.options.length - 1; m >= 0; m--) { + fooTemp.remove(m); + } + + for (var i = 0; i < fooGroup[x].length; i++) { + fooTemp.options[i] = new Option(fooGroup[x][i].text, fooGroup[x][i].value); + for (var j = 0; j < fooSelected.length; j++) { + if (fooTemp.options[i].value == fooSelected[j]) { + fooTemp.options[i].selected = true; + selected = true; + } + } + } + + if ((fooTemp.options.length > 0) && (! selected)) { + fooTemp.options[0].selected = true; + } + } +</script> + </td> +</tr>
