This is an automated email from the ASF dual-hosted git repository. reiern70 pushed a commit to branch wicket-10.x in repository https://gitbox.apache.org/repos/asf/wicket.git
commit fc5c15034b9b59f1fe9ce5ee09fb1504d57bd8dc Author: reiern70 <[email protected]> AuthorDate: Sat Sep 26 09:26:33 2026 -0500 Fix Palette move up throwing TypeError on plain DOM element Wicket.Palette.moveUpHelper called box.trigger('focus'), but box is a plain DOM <select> element returned by document.getElementById, not a jQuery object. Clicking the "move up" button on an ordered Palette threw a TypeError before reaching updateRecorder, so the hidden recorder input never picked up the new order and the reordering was discarded on the next render. The "move down" button was unaffected since moveDownHelper has no equivalent call. This was introduced by the jQuery 4.0.0 migration, which rewrote box.focus() as box.trigger('focus'). master is unaffected because it later dropped jQuery from this file entirely and regained box.focus() incidentally. This fix restores box.focus() on wicket-10.x, matching what master does today. Adds a QUnit harness for wicket-extensions' client-side JavaScript (none existed; only wicket-core's was wired into the grunt/js-test build) and a regression test asserting that moveUp reorders the selection and updates the recorder without throwing. GitHub issue #1618 --- testing/wicket-js-tests/Gruntfile.js | 17 ++++- .../extensions/markup/html/form/palette/palette.js | 2 +- wicket-extensions/src/test/js/palette-test.js | 88 ++++++++++++++++++++++ wicket-extensions/src/test/js/palette.html | 54 +++++++++++++ 4 files changed, 159 insertions(+), 2 deletions(-) diff --git a/testing/wicket-js-tests/Gruntfile.js b/testing/wicket-js-tests/Gruntfile.js index 6ee82e34ce..62ea0c8fe0 100644 --- a/testing/wicket-js-tests/Gruntfile.js +++ b/testing/wicket-js-tests/Gruntfile.js @@ -45,6 +45,9 @@ module.exports = function(grunt) { "../../wicket-core/src/test/js/event.js", "../../wicket-core/src/test/js/timer.js" ], + extensionsTestsJs = [ + "../../wicket-extensions/src/test/js/palette-test.js" + ], gymTestsJs = [ "../../wicket-examples/src/main/webapp/js-test/tests/ajax/form.js", "../../wicket-examples/src/main/webapp/js-test/tests/bean-validation/birthdate.js", @@ -72,6 +75,7 @@ module.exports = function(grunt) { extensions: extensionsJs, nativeWebSocket: nativeWebSocketJs, testsJs: testsJs, + extensionsTestsJs: extensionsTestsJs, gymTestsJs: gymTestsJs, grunt: gruntJs, @@ -107,7 +111,9 @@ module.exports = function(grunt) { options: { urls: [ 'http://localhost:38887/test/js/all.html?4.0.0', - 'http://localhost:38887/test/js/all.html?3.7.1' + 'http://localhost:38887/test/js/all.html?3.7.1', + 'http://localhost:38888/wicket-extensions/src/test/js/palette.html?4.0.0', + 'http://localhost:38888/wicket-extensions/src/test/js/palette.html?3.7.1' ], puppeteer: { headless: true, @@ -135,6 +141,15 @@ module.exports = function(grunt) { }, base: '../../wicket-core/src' } + }, + // serves wicket-extensions' own JavaScript and its QUnit tests; a separate + // target because they live outside the wicket-core/src root above + extensions: { + options: { + port: 38888, + debug: true, + base: '../..' + } } } }); diff --git a/wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/form/palette/palette.js b/wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/form/palette/palette.js index 16df1386fd..16ec0148f8 100644 --- a/wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/form/palette/palette.js +++ b/wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/form/palette/palette.js @@ -86,7 +86,7 @@ if(!box.options[i-1].selected) { box.insertBefore(box.options[i],box.options[i-1]); dirty=true; - box.trigger('focus'); + box.focus(); } } } diff --git a/wicket-extensions/src/test/js/palette-test.js b/wicket-extensions/src/test/js/palette-test.js new file mode 100644 index 0000000000..6f8c651c58 --- /dev/null +++ b/wicket-extensions/src/test/js/palette-test.js @@ -0,0 +1,88 @@ +/* + * 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. + */ + +/*global QUnit: true */ + +jQuery(document).ready(function() { + "use strict"; + + const { module, test } = QUnit; + + function selectionOptionValues() { + var selection = document.getElementById('paletteSelection'); + return Array.prototype.map.call(selection.options, function(option) { + return option.value; + }); + } + + function selectOnly(selection, value) { + Array.prototype.forEach.call(selection.options, function(option) { + option.selected = (option.value === value); + }); + } + + module("Wicket.Palette", { + beforeEach: function() { + var selection = document.getElementById('paletteSelection'); + selection.innerHTML = + '<option value="1">one</option>' + + '<option value="2">two</option>' + + '<option value="3">three</option>'; + document.getElementById('paletteRecorder').value = '1,2,3'; + } + }); + + test("moveUp reorders the selected option without throwing", assert => { + var selection = document.getElementById('paletteSelection'); + selectOnly(selection, '3'); + + Wicket.Palette.moveUp('paletteChoices', 'paletteSelection', 'paletteRecorder'); + + assert.deepEqual(selectionOptionValues(), ['1', '3', '2'], + "moveUp did not move the selected option in front of its predecessor"); + }); + + test("moveUp updates the hidden recorder input", assert => { + var selection = document.getElementById('paletteSelection'); + selectOnly(selection, '3'); + + Wicket.Palette.moveUp('paletteChoices', 'paletteSelection', 'paletteRecorder'); + + assert.equal(document.getElementById('paletteRecorder').value, '1,3,2', + "the recorder was not updated, so the server never sees the new order"); + }); + + test("moveUp on the first option is a no-op and does not throw", assert => { + var selection = document.getElementById('paletteSelection'); + selectOnly(selection, '1'); + + Wicket.Palette.moveUp('paletteChoices', 'paletteSelection', 'paletteRecorder'); + + assert.deepEqual(selectionOptionValues(), ['1', '2', '3'], + "the order should not have changed"); + }); + + test("moveDown reorders the selected option without throwing", assert => { + var selection = document.getElementById('paletteSelection'); + selectOnly(selection, '1'); + + Wicket.Palette.moveDown('paletteChoices', 'paletteSelection', 'paletteRecorder'); + + assert.deepEqual(selectionOptionValues(), ['2', '1', '3'], + "moveDown did not move the selected option behind its successor"); + }); +}); diff --git a/wicket-extensions/src/test/js/palette.html b/wicket-extensions/src/test/js/palette.html new file mode 100644 index 0000000000..f9a04150f7 --- /dev/null +++ b/wicket-extensions/src/test/js/palette.html @@ -0,0 +1,54 @@ +<?xml version="1.0" encoding="UTF-8" ?> +<!-- + 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. +--> +<html> + +<head> + <title id="titleId">Wicket.Palette JavaScript tests</title> + <meta http-equiv="content-type" content="text/html; charset=UTF-8"> + <link rel="stylesheet" href="/wicket-core/src/test/js/qunit/qunit.css" type="text/css" media="screen" /> +</head> + +<body> + <div id="qunit"></div> + + <div id="qunit-fixture"> + <select id="paletteChoices" multiple="multiple"> + <option value="4">four</option> + </select> + <select id="paletteSelection" multiple="multiple"> + <option value="1">one</option> + <option value="2">two</option> + <option value="3">three</option> + </select> + <input type="hidden" id="paletteRecorder" value="1,2,3"/> + </div> + + <script> + // version lies between question mark and first ampersand (or end) + var version = location.search.match(/\?(.*?)(&|$)/)[1]; + + document.write("<scr"+"ipt src='/wicket-core/src/main/java/org/apache/wicket/resource/jquery/jquery-"+version+".js'></scr"+"ipt>"); + </script> + <script type="text/javascript" src="/wicket-core/src/test/js/qunit/qunit.js"></script> + + <!-- the module under test --> + <script type="text/javascript" src="/wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/form/palette/palette.js"></script> + + <script type="text/javascript" src="palette-test.js"></script> +</body> +</html>
