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

[aeonframework]

Fixes WW-5772

Problem

The optiontransferselect, inputtransferselect and updownselect tags render every button with an onclick handler that the framework builds itself. The combobox tag renders its select with an inline onChange. An application that wants a nonce-based Content-Security-Policy without 'unsafe-inline' therefore cannot use these tags, even though <s:script> already applies the nonce to the scripts the same templates emit.

The generated onclick code also contains the headerKey/doubleHeaderKey values as JavaScript string literals that are only HTML-escaped. The browser decodes ' back to ' before it runs the handler, so a header key that contains a quote breaks the handler.

Approach

• Transfer selects and updownselect. Each button now carries its action, the target select id(s) and the header key(s) in data-struts-* attributes, which FreeMarker HTML-escapes. optiontransferselect.js (also used by updownselect) and inputtransferselect.js each register one delegated click listener per page, in the capture phase, guarded against double registration. The listener calls the same existing functions with the same arguments as the old inline handlers, header-key/no-header-key branches included. optiontransferselect.ftl folds its duplicated per-button branches into one transferButton macro, about 100 lines shorter. A developer-supplied *Onclick attribute is still rendered as onclick and still runs after the framework action.
• Combobox (simple and html5). The select carries data-struts-combobox="<escapedId>". The change listener is attached from an <@s.script> block, which carries the nonce like the autoPopulate_<id> function the template already emits. The script binds every select with that marker that is not bound yet. Repeated comboboxes with the same id (e.g. inside an iterator) therefore each get exactly one handler, and fragments loaded over AJAX work too: jQuery .html() evaluates scripts through a temporary element in <head>, so document.currentScript cannot be used.
• The old global JS functions are unchanged, so custom themes that copied the old templates keep working.

Why it is systematic

Each of these tags had the same pattern: framework-built JavaScript in event-handler attributes. The patch removes that pattern from them entirely rather than escaping individual values, so neither the CSP blocker nor the quoting problem can come back through these templates.

Upgrade note (please add to the release notes / migration guide)

The new markup needs the new optiontransferselect.js and inputtransferselect.js.

• An application that serves its own copy of the static files (struts.serve.static=false or a custom struts.ui.staticContentPath) must update those two files together with the templates. Otherwise the transfer buttons silently do nothing.
• The static URL is not versioned and browsers cache it for one day, so a browser holding the old copy right after an upgrade sees inactive buttons until the cache expires.
• Code that calls button.onclick() directly no longer triggers the framework action. Real clicks and element.click() still do.

Tests

• ./mvnw -pl core test on JDK 17: 3466 tests, 0 failures, 0 errors, 0 skipped.
• New tests in OptionTransferSelectTagTest and UpDownSelectTagTest check that framework buttons render no onclick and that a header key containing a quote (it's-left) lands HTML-escaped in a data-* attribute. The rendering fixtures are updated. On the base templates, 51 of the 57 tests in the four affected tag test classes fail.
• Manual check in headless Chromium, under <meta http-equiv="Content-Security-Policy" content="script-src 'nonce-…'"> with no 'unsafe-inline': zero CSP violations. These all worked:
  • optiontransferselect add-to-right, add-all-to-left and select-all, with the header options kept in place;
  • updownselect move-up and select-all;
  • inputtransferselect add, and move-up stopping at the header key;
  • two comboboxes with the same id in two forms, each populating only its own field;
  • a combobox inserted with innerHTML and its scripts evaluated jQuery-style, which populated its field.

Follow-ups (separate tickets; this PR does not make the themes fully CSP-clean)

• xhtml/css_xhtml form.ftl: default onreset="clearErrorMessages…" and the onsubmit="return validateForm_…" for client-side validation
• xhtml tooltip.ftl: domTT_activate onmouseover when javascriptTooltip is on
• doubleselect: the onchange="<id>Redirect(…)" built in DoubleSelect.java

This change was prepared with AI assistance and reviewed and tested by me.

[lukaszlenart]

Thanks for this, moving the framework-built handlers out of the markup is a good direction. A few points below. None of them block this PR; I've filed the larger ones as follow-up tickets so they don't need to be folded in here:

• WW-5773: version the static script URLs emitted by tag templates
• WW-5774: move combobox change handling into a static script with a delegated listener
• WW-5775: intent tests for inputtransferselect and combobox

[lukaszlenart]

The new markup only works with the new optiontransferselect.js, but this URL isn't versioned. A browser holding the cached old script (static content is cached for a day), or an app serving its own copy, renders buttons with no onclick and no listener, so clicks silently do nothing. Same for inputtransferselect.ftl and updownselect.ftl.

The upgrade note in WW-5772 covers it for now; a cache-busting parameter on these URLs is tracked separately in WW-5773.

[lukaszlenart]

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.

[lukaszlenart]

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.

[lukaszlenart]

Nice targeted test. There's no equivalent for inputtransferselect (data-struts-header-key on move up/down) or for combobox (no inline onChange, marker attribute, nonce on the binding script), so regressions there are only caught by the fixture files. Tracked in WW-5775.