You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by re...@apache.org on 2026/09/26 14:06:17 UTC
(wicket) branch reiern70/issue-1618-10x-broken-palette created (now eb415ef6c2)
This is an automated email from the ASF dual-hosted git repository.
reiern70 pushed a change to branch reiern70/issue-1618-10x-broken-palette
in repository https://gitbox.apache.org/repos/asf/wicket.git
at eb415ef6c2 Fix TrapFocusBehavior throwing TypeError when Tab wraps focus
This branch includes the following new commits:
new b80a03deca Fix Palette move up throwing TypeError on plain DOM element
new eb415ef6c2 Fix TrapFocusBehavior throwing TypeError when Tab wraps focus
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(wicket) 02/02: Fix TrapFocusBehavior throwing TypeError when Tab wraps focus
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
reiern70 pushed a commit to branch reiern70/issue-1618-10x-broken-palette
in repository https://gitbox.apache.org/repos/asf/wicket.git
commit eb415ef6c2f5dfa071185a5795c993c2f3503ad0
Author: reiern70 <re...@gmail.com>
AuthorDate: Sat Sep 26 09:06:01 2026 -0500
Fix TrapFocusBehavior throwing TypeError when Tab wraps focus
Wicket.trapFocus's keydown handler resolves the first and last
focusable elements with $focusable.get(0) and
$focusable.get($focusable.length - 1), which are plain DOM elements,
then called .trigger('focus') on them. Tabbing past the last
focusable element (or Shift+Tabbing past the first) inside a
focus-trapped container - e.g. a ModalDialog - threw a TypeError
instead of wrapping focus around, so keyboard users got stuck instead
of cycling back to the other end of the trap.
The other two calls in the same file already went through
findFocusable(...).first(), which returns a jQuery object, so they
were unaffected. This is the same class of bug as the Palette "move
up" issue: both were introduced by the jQuery 4.0.0 migration, which
mechanically rewrote box.focus() as box.trigger('focus') without
checking whether the receiver was a jQuery object. master does not
have this bug; it later dropped jQuery from this file and uses
.focus() directly.
Adds a QUnit regression test asserting that Tab/Shift+Tab at the
trap's boundaries move focus to the other end without throwing.
GitHub issue #1618
Co-Authored-By: Claude Sonnet 5 <no...@anthropic.com>
---
testing/wicket-js-tests/Gruntfile.js | 7 +-
.../ajax/markup/html/modal/trap-focus.js | 4 +-
wicket-extensions/src/test/js/trap-focus.html | 34 ++++++++++
wicket-extensions/src/test/js/trapfocus-test.js | 75 ++++++++++++++++++++++
4 files changed, 116 insertions(+), 4 deletions(-)
diff --git a/testing/wicket-js-tests/Gruntfile.js b/testing/wicket-js-tests/Gruntfile.js
index 62ea0c8fe0..c89f34e316 100644
--- a/testing/wicket-js-tests/Gruntfile.js
+++ b/testing/wicket-js-tests/Gruntfile.js
@@ -46,7 +46,8 @@ module.exports = function(grunt) {
"../../wicket-core/src/test/js/timer.js"
],
extensionsTestsJs = [
- "../../wicket-extensions/src/test/js/palette-test.js"
+ "../../wicket-extensions/src/test/js/palette-test.js",
+ "../../wicket-extensions/src/test/js/trapfocus-test.js"
],
gymTestsJs = [
"../../wicket-examples/src/main/webapp/js-test/tests/ajax/form.js",
@@ -113,7 +114,9 @@ module.exports = function(grunt) {
'http://localhost:38887/test/js/all.html?4.0.0',
'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'
+ 'http://localhost:38888/wicket-extensions/src/test/js/palette.html?3.7.1',
+ 'http://localhost:38888/wicket-extensions/src/test/js/trap-focus.html?4.0.0',
+ 'http://localhost:38888/wicket-extensions/src/test/js/trap-focus.html?3.7.1'
],
puppeteer: {
headless: true,
diff --git a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/trap-focus.js b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/trap-focus.js
index d8b49822ac..ffa8b137fa 100644
--- a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/trap-focus.js
+++ b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/trap-focus.js
@@ -70,12 +70,12 @@
if (e.shiftKey) {
if (e.target === firstFocusable || $element.is(e.target)) {
e.preventDefault();
- lastFocusable.trigger('focus');
+ lastFocusable.focus();
}
} else {
if (e.target === lastFocusable || $element.is(e.target)) {
e.preventDefault();
- firstFocusable.trigger('focus');
+ firstFocusable.focus();
}
}
}
diff --git a/wicket-extensions/src/test/js/trap-focus.html b/wicket-extensions/src/test/js/trap-focus.html
new file mode 100644
index 0000000000..099a35d8d9
--- /dev/null
+++ b/wicket-extensions/src/test/js/trap-focus.html
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<html>
+
+<head>
+ <title id="titleId">Wicket.trapFocus 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">
+ <div id="trapFocusContainer">
+ <a id="trapFocusFirst" href="#first">first</a>
+ <a id="trapFocusLast" href="#last">last</a>
+ </div>
+ </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 src="/wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js"></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/ajax/markup/html/modal/trap-focus.js"></script>
+
+ <script type="text/javascript" src="trapfocus-test.js"></script>
+</body>
+</html>
diff --git a/wicket-extensions/src/test/js/trapfocus-test.js b/wicket-extensions/src/test/js/trapfocus-test.js
new file mode 100644
index 0000000000..ffecf2e0ff
--- /dev/null
+++ b/wicket-extensions/src/test/js/trapfocus-test.js
@@ -0,0 +1,75 @@
+/*
+ * 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;
+
+ const TAB_KEYCODE = 9;
+
+ function tab(target, shiftKey) {
+ jQuery(target).trigger(jQuery.Event('keydown', { which: TAB_KEYCODE, keyCode: TAB_KEYCODE, shiftKey: !!shiftKey }));
+ }
+
+ module("Wicket.trapFocus", {
+ afterEach: function() {
+ // detach the trap's global 'focusin' handler and reset state between tests
+ jQuery(document.getElementById('trapFocusContainer')).trigger('trapfocusremove');
+ }
+ });
+
+ test("Tab on the last focusable element wraps focus to the first, without throwing", assert => {
+ window.Wicket.trapFocus('trapFocusContainer', 'wicket-trap-focus-test');
+
+ var first = document.getElementById('trapFocusFirst');
+ var last = document.getElementById('trapFocusLast');
+ last.focus();
+
+ var threw = false;
+ try {
+ tab(last, false);
+ } catch (e) {
+ threw = true;
+ }
+
+ assert.notOk(threw, "tabbing past the last focusable element threw an exception");
+ assert.equal(document.activeElement, first,
+ "focus did not wrap around to the first focusable element");
+ });
+
+ test("Shift+Tab on the first focusable element wraps focus to the last, without throwing", assert => {
+ window.Wicket.trapFocus('trapFocusContainer', 'wicket-trap-focus-test');
+
+ var first = document.getElementById('trapFocusFirst');
+ var last = document.getElementById('trapFocusLast');
+ first.focus();
+
+ var threw = false;
+ try {
+ tab(first, true);
+ } catch (e) {
+ threw = true;
+ }
+
+ assert.notOk(threw, "shift-tabbing past the first focusable element threw an exception");
+ assert.equal(document.activeElement, last,
+ "focus did not wrap around to the last focusable element");
+ });
+});
(wicket) 01/02: Fix Palette move up throwing TypeError on plain DOM element
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
reiern70 pushed a commit to branch reiern70/issue-1618-10x-broken-palette
in repository https://gitbox.apache.org/repos/asf/wicket.git
commit b80a03deca3c2ab7fbd2f0d3335af7b9869918bb
Author: reiern70 <re...@gmail.com>
AuthorDate: Sat Sep 26 09:05:32 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
Co-Authored-By: Claude Sonnet 5 <no...@anthropic.com>
---
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 | 38 ++++++++++
4 files changed, 143 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..306a4a2eba
--- /dev/null
+++ b/wicket-extensions/src/test/js/palette.html
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<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>