From 3187565d004439a2e1d4d04a0ae86e83d39f1be2 Mon Sep 17 00:00:00 2001
From: Ilia Ross <ilia@ross.gdn>
Date: Wed, 9 Sep 2026 01:05:07 +0200
Subject: [PATCH] Fix legacy compatibility in multi-selection lists

---
 t/ui-lib-widgets.t             | 99 ++++++++++++++++++++++++++++++++++
 ui-lib.pl                      | 67 +++++++++++++++++------
 unauthenticated/css/ui-lib.css |  5 ++
 unauthenticated/js/ui-lib.js   | 23 ++++++--
 4 files changed, 176 insertions(+), 18 deletions(-)

diff --git a/t/ui-lib-widgets.t b/t/ui-lib-widgets.t
index 0835df664..0cdf3cf86 100644
--- a/t/ui-lib-widgets.t
+++ b/t/ui-lib-widgets.t
@@ -403,6 +403,105 @@ like(main::ui_form_columns_table('x.cgi', [ [ 'go', 'Go' ] ], 0, undef, undef,
 		'disabled of ui_multi_select disables the rows');
 }
 
+# Legacy attributes must retain row presentation and checkbox behavior.
+{
+	my $tags = q{DISABLED="false" class="managed" style='font-style:italic' title="Policy &amp; owner" data-policy=managed};
+	my @options = ( [ 'locked', 'Locked', $tags ],
+		[ 'open', 'Open', q{title="This is not disabled" data-note='a=b > c'} ] );
+	my $html = main::ui_multi_select_list('attrs', [ 'locked' ], \@options);
+	like($html, qr/<input (?=[^>]*value="locked")(?=[^>]*\bchecked\b)(?=[^>]*\bdisabled\b)/,
+		'legacy boolean disabled locks a selected checkbox, even with value false');
+	like($html, qr/name="attrs"[^>]*value="locked"/,
+		'disabled selections retain their submitted values');
+	like($html, qr/<div (?=[^>]*\bui_multi_item\b)(?=[^>]*\bui_multi_disabled\b)(?=[^>]*\bmanaged\b)/,
+		'legacy classes combine with the widget and disabled row classes');
+	like($html, qr/<div (?=[^>]*\bui_multi_item\b)(?=[^>]*style="font-style:italic")/,
+		'legacy styles apply to the whole row');
+	like($html, qr/title="Policy &amp; owner"/,
+		'legacy title entities are preserved without double escaping');
+	like($html, qr/data-policy="managed"/,
+		'unquoted data attributes survive normalization');
+	unlike($html, qr/<input (?=[^>]*value="open")(?=[^>]*\bdisabled\b)/,
+		'disabled inside a quoted attribute does not disable the checkbox');
+	like($html, qr/data-note="a=b > c"/,
+		'quoted attribute values retain spaces, equals and greater-than signs');
+	is($options[0]->[2], $tags, 'normalization does not modify legacy tags');
+	foreach my $disabled ( 'disabled', "disabled='disabled'", 'disabled=disabled' ) {
+		like(main::ui_multi_select_list('d', [ ], [ [ 'a', 'A', $disabled ] ]),
+			qr/<input (?=[^>]*value="a")(?=[^>]*\bdisabled\b)/,
+			"checkbox honors $disabled");
+		}
+	my $attrs = { 'disabled' => undef, 'title' => 'Managed', 'class' => 'managed' };
+	my $hash = main::ui_multi_select_list('hashattrs', [ ],
+		[ { 'value' => 'a', 'label' => 'A', 'attrs' => $attrs } ]);
+	like($hash, qr/<input (?=[^>]*value="a")(?=[^>]*\bdisabled\b)/,
+		'hash attributes also disable the checkbox');
+	is_deeply($attrs, { 'disabled' => undef, 'title' => 'Managed', 'class' => 'managed' },
+		'normalization does not modify the caller attribute hash');
+	my $missing = main::ui_multi_select_list('missing',
+		[ [ 'gone', 'Gone', q{disabled title='Retained'} ] ], [ ]);
+	like($missing, qr/<input (?=[^>]*value="gone")(?=[^>]*\bchecked\b)(?=[^>]*\bdisabled\b)/,
+		'missing selected values retain their legacy disabled attribute');
+	like($missing, qr/title="Retained"/, 'missing values retain row attributes');
+	my $fixed = main::ui_multi_select_list('fixed', [ ],
+		[ [ 'a', 'A', q{checked name=wrong value=wrong type=radio} ] ]);
+	like($fixed, qr/<input (?=[^>]*type='checkbox')(?=[^>]*name="fixed_item")(?=[^>]*value="a")/,
+		'row attributes cannot replace checkbox identity');
+	unlike($fixed, qr/<input (?=[^>]*value="a")(?=[^>]*\bchecked\b)/,
+		'row attributes cannot override the selected values');
+	my @disabled;
+	{
+		no warnings qw(redefine once);
+		local *main::theme_ui_checkbox = sub { push(@disabled, $_[5]); return ''; };
+		main::ui_multi_select_list('themed', [ ], \@options);
+	}
+	is_deeply(\@disabled, [ 1, 0 ], 'themes receive the normalized disabled state');
+}
+
+# Positional calls preserve the old selected pane's labels and value order.
+{
+	my @values = ( [ 'b', 'B (selected)', q{disabled title="Selected"} ],
+		[ 'a', 'R&amp;D' ], [ 'gone', 'Missing', q{title="Retained"} ] );
+	my @options = ( [ 'a', 'A', 'disabled' ], [ 'b', 'B' ],
+		[ 'c', 'C', 'disabled' ] );
+	my $html = main::ui_multi_select_list('legacy', \@values, \@options, 5, 1);
+	my $old = main::ui_multi_select('legacy', \@values, \@options, 5);
+	my ($value) = $html =~ /type='hidden'[^>]*name="legacy"[^>]*value="([^"]*)"/;
+	my ($oldvalue) = $old =~ /type='hidden'[^>]*name="legacy"[^>]*value="([^"]*)"/;
+	is($value, $oldvalue, 'legacy submitted order matches the old widget');
+	my ($order) = $html =~ /data-ui-multi-order="([^"]*)"/;
+	is($order, '', 'legacy order is captured from the initial hidden input');
+	like($html, qr/>B \(selected\)</, 'legacy selected description overrides the option label');
+	like($html, qr/>R&amp;D</, 'legacy pre-escaped labels are not double escaped');
+	my ($filter) = $html =~ /data-ui-multi-text="(R[^"]*)"/;
+	is(decode_attr($filter), 'R&D', 'legacy filter text matches the visible label');
+	like($html, qr/<input (?=[^>]*value="b")(?=[^>]*\bdisabled\b)/,
+		'legacy selected attributes disable existing entries');
+	unlike($html, qr/<input (?=[^>]*value="a")(?=[^>]*\bdisabled\b)/,
+		'selected attributes take precedence over available option attributes');
+	like($html, qr/<input (?=[^>]*value="c")(?=[^>]*\bdisabled\b)/,
+		'unselected entries keep their option attributes');
+	like($html, qr/title="Selected"/, 'legacy selected row keeps its tooltip');
+	like($html, qr/title="Retained"/, 'missing legacy selection keeps its tooltip');
+	is_deeply(\@values, [ [ 'b', 'B (selected)', q{disabled title="Selected"} ],
+		[ 'a', 'R&amp;D' ], [ 'gone', 'Missing', q{title="Retained"} ] ],
+		'legacy selected entries are not modified');
+	is_deeply(\@options, [ [ 'a', 'A', 'disabled' ], [ 'b', 'B' ],
+		[ 'c', 'C', 'disabled' ] ], 'legacy options are not modified');
+	my $numeric = main::ui_multi_select_list('numeric', [ [ 2, 'Two' ], [ 1, 'One' ] ],
+		[ [ 1, 'One' ], [ 2, 'Two' ] ], 5);
+	like($numeric, qr/name="numeric"[^>]*value="2\n1"/,
+		'numeric selection IDs retain their original order');
+	assert_no_handler_injection(main::ui_multi_select_list('legacy_xss',
+		[ [ 'a', $xss ] ], [ [ 'a', 'A' ] ], 5), 'legacy selected label');
+	my $modern = main::ui_multi_select_list('modern', [ [ 'b', 'B.x' ], [ 'a', 'A' ] ],
+		[ [ 'a', 'R&amp;D' ], { 'value' => 'b', 'label' => 'B', 'suffix' => '.x' } ], {});
+	unlike($modern, qr/data-ui-multi-order=/, 'options-hash calls retain the modern ordering policy');
+	like($modern, qr/name="modern"[^>]*value="a\nb"/, 'modern selections follow option order');
+	like($modern, qr/>R&amp;amp;D</, 'modern labels retain literal entity text');
+	like($modern, qr/>B<\/label><span[^>]*>\.x</, 'modern option labels retain their suffix without duplication');
+}
+
 # Preserve UTF-8 bytes and literal entities for browser-side matching.
 {
 	my $label = "\xC3\x89QUIPE";
diff --git a/ui-lib.pl b/ui-lib.pl
index 128f8a54a..a85eb6806 100755
--- a/ui-lib.pl
+++ b/ui-lib.pl
@@ -5700,7 +5700,8 @@ return &_ui_block('div', $select.$panels, &_ui_attrs({
 Returns a scrolling checkbox list with selection links, an expandable filter
 and a selection count beside the mode selector or links. Like ui_multi_select,
 it submits newline-joined values under name; ui-lib.js keeps them in sync.
-Missing selected values are added automatically. Labels are plain text.
+Missing selected values are added automatically. Labels are plain text with
+the options-hash API; positional calls also accept pre-escaped labels.
 
 With no entries, it shows only empty_label, preserving the selection, mode
 and children form values in hidden inputs. Nonempty lists load their assets
@@ -5711,14 +5712,15 @@ current one, skipping filtered, folded and disabled entries. Labels and row
 backgrounds work too.
 
 Accepts an options hash or ui_multi_select's trailing positional arguments.
-Only disabled is used from the legacy arguments; size, add-if-missing,
-titles and width are ignored.
+Positional calls preserve selected labels, attributes, value order and
+pre-escaped label text. Newly selected entries are prepended as in the old
+widget. Size, add-if-missing, titles and width are ignored.
 
 =item name - HTML name for the input.
 
-=item values - Array reference of selected scalars or [ value, label ] pairs.
+=item values - Array reference of selected scalars or [ value, label, attributes ] entries. Attributes are also retained when a selected entry is missing from options.
 
-=item options - Array reference of [ value, label ] pairs or hashes with keys value, label, suffix (muted text after the label), level (indentation depth), tag (chip at the right) and disabled.
+=item options - Array reference of [ value, label, attributes ] entries or hashes with keys value, label, suffix (muted text after the label), level (indentation depth), tag (chip at the right), disabled and attrs. Attributes may be a hash or trusted HTML attribute string, as in ui_select. disabled applies to the checkbox; other attributes, such as title, style and class, apply to the row. Widget identity and selection remain controlled by the library.
 
 =item opts - Optional hash reference with the keys :
 
@@ -5746,6 +5748,7 @@ sub ui_multi_select_list
 return &theme_ui_multi_select_list(@_)
 	if (defined(&theme_ui_multi_select_list));
 my ($name, $values, $options, $opts) = @_;
+my $legacy = @_ > 3 && ref($opts) ne 'HASH';
 if (ref($opts) ne 'HASH') {
 	# Accept ui_multi_select's positional disabled argument.
 	$opts = { 'disabled' => $_[5] };
@@ -5760,25 +5763,56 @@ foreach my $o (@{$options || []}) {
 		push(@items, { %$o });
 		}
 	elsif (ref($o) eq 'ARRAY') {
-		push(@items, { 'value' => $o->[0], 'label' => $o->[1] });
+		push(@items, { 'value' => $o->[0], 'label' => $o->[1],
+			      'attrs' => $o->[2] });
 		}
 	}
 # Index options to add missing selections without repeated scans.
-my %offered = map { defined($_->{'value'}) ? ($_->{'value'}, 1) : () } @items;
+my %offered = map { defined($_->{'value'}) ? ($_->{'value'}, $_) : () } @items;
 my %selected;
+my @chosen;
 foreach my $v (@{$values || []}) {
-	my ($val, $label) = ref($v) eq 'ARRAY' ? @$v : ($v);
+	my ($val, $label, $attrs) = ref($v) eq 'ARRAY' ? @$v : ($v);
 	next if (!defined($val));
-	$selected{$val} = 1;
-	push(@items, { 'value' => $val, 'label' => $label })
-		if (!$offered{$val}++);
+	next if ($selected{$val}++);
+	my $item = $offered{$val};
+	if (!$item) {
+		$item = { 'value' => $val, 'label' => $label, 'attrs' => $attrs };
+		push(@items, $item);
+		$offered{$val} = $item;
+		}
+	# The old selected pane uses the label and attributes supplied in values.
+	if ($legacy && ref($v) eq 'ARRAY') {
+		$item->{'label'} = defined($label) && $label ne '' ? $label : $val;
+		$item->{'attrs'} = $attrs;
+		}
+	push(@chosen, $item);
 	}
 foreach my $it (@items) {
 	$it->{'value'} = '' if (!defined($it->{'value'}));
 	$it->{'label'} = $it->{'value'}
 		if (!defined($it->{'label'}) || $it->{'label'} eq '');
+	my $tags = $it->{'attrs'};
+	my %attrs;
+	if (ref($tags) eq 'HASH') {
+		# Copy and normalize attribute names without changing caller data.
+		%attrs = map { lc($_), $tags->{$_} } keys %$tags;
+		}
+	else {
+		# Parse legacy tags without splitting quoted values.
+		$tags ||= '';
+		while ($tags =~ /\G\s*([a-z_:][a-z0-9_.:-]*)(?:\s*=\s*(?:"([^"]*)"|'([^']*)'|([^\s"'<>`]+)))?/gci) {
+			my ($name, $double, $single, $bare) = ($1, $2, $3, $4);
+			$attrs{lc($name)} = defined($double) ? $double :
+					   defined($single) ? $single : $bare;
+			}
+		}
+	# HTML boolean attributes are true whenever present, even disabled="false".
+	$it->{'disabled'} ||= exists($attrs{'disabled'});
+	delete($attrs{'disabled'});
+	$it->{'attrs'} = \%attrs;
 	}
-my @chosen = grep { $selected{$_->{'value'}} } @items;
+@chosen = grep { $selected{$_->{'value'}} } @items if (!$legacy);
 
 # Resolve the mode selector and initial list visibility.
 my $modes = $opts->{'modes'};
@@ -5902,7 +5936,7 @@ my $body = &ui_tag('div', $tools, { 'class' => 'ui_multi_tools' });
 my $rows = "";
 foreach my $it (@items) {
 	my $val = $it->{'value'};
-	my $label = &html_escape($it->{'label'});
+	my $label = &html_escape($it->{'label'}, $legacy);
 	$label .= &ui_tag('span', &html_escape($it->{'suffix'}),
 			  { 'class' => 'ui_multi_suffix' })
 		if (defined($it->{'suffix'}) && $it->{'suffix'} ne '');
@@ -5920,8 +5954,9 @@ foreach my $it (@items) {
 			{ 'class' => 'ui_multi_side' })
 		if (defined($it->{'tag'}) && $it->{'tag'} ne '');
 	# Lowercase in the browser, after UTF-8 bytes have been decoded.
-	my $attrs = &_ui_attrs({
+	my $attrs = { %{$it->{'attrs'}}, %{&_ui_attrs({
 		'class' => &_ui_class('ui_multi_item',
+			$it->{'attrs'}->{'class'},
 			$it->{'level'} ? 'ui_multi_level'.int($it->{'level'})
 				       : undef,
 			$it->{'disabled'} ? 'ui_multi_disabled' : undef),
@@ -5929,7 +5964,7 @@ foreach my $it (@items) {
 						       : undef,
 		'data-ui-multi-text' => &html_escape(join(" ",
 			grep { defined($_) && $_ ne '' }
-			     $it->{'label'}.($it->{'suffix'} // ''), $it->{'tag'})) });
+			     $it->{'label'}.($it->{'suffix'} // ''), $it->{'tag'}), $legacy) }) } };
 	$attrs->{'hidden'} = undef if ($folded && $it->{'level'});
 	$rows .= &ui_tag('div', $row, $attrs);
 	}
@@ -5971,6 +6006,8 @@ my $attrs = &_ui_attrs({
 	'data-ui-multi-hide' => &html_escape(
 		&convert_to_json([ map { "$_" } @hide ])),
 	'data-ui-multi-text-selected' => $text{'ui_multi_selected'} });
+# Have JavaScript retain the hidden input's initial order for resets.
+$attrs->{'data-ui-multi-order'} = '' if ($legacy);
 return &ui_page_assets().&_ui_block('div', $rv, $attrs);
 }
 
diff --git a/unauthenticated/css/ui-lib.css b/unauthenticated/css/ui-lib.css
index 2be09ebc9..f270f1df9 100644
--- a/unauthenticated/css/ui-lib.css
+++ b/unauthenticated/css/ui-lib.css
@@ -1104,6 +1104,11 @@ a.ui_list_link:hover { color: var(--ui-accent) !important; }
 }
 /* Space unwrapped checkboxes; theme wrappers handle their own spacing. */
 .ui_multi_item > input[type="checkbox"] { margin-right: 6px; }
+/* Keep themed checkbox glyphs upright when the row text is italic. */
+.ui_multi_item input[type="checkbox"] + label::before,
+.ui_multi_item input[type="checkbox"] + label::after {
+	font-style: normal;
+}
 .ui_multi_item:hover { background: var(--ui-surface-2); }
 .ui_multi_level1 { padding-left: 27px; }
 .ui_multi_level2 { padding-left: 47px; }
diff --git a/unauthenticated/js/ui-lib.js b/unauthenticated/js/ui-lib.js
index 01f74bffb..c82d22bdd 100644
--- a/unauthenticated/js/ui-lib.js
+++ b/unauthenticated/js/ui-lib.js
@@ -121,7 +121,7 @@
 		applyMulti(box);
 		(open ? search : filter.querySelector('.ui_multi_filter_toggle')).focus();
 	}
-	function applyMulti(box) {
+	function applyMulti(box, reset) {
 		// Resets and history restores need not fire change events.
 		var mode = box.querySelector('.ui_multi_modes select, .ui_multi_modes input[type="radio"]:checked');
 		var body = box.querySelector('.ui_multi_body');
@@ -171,7 +171,24 @@
 		}
 		var name = box.getAttribute('data-ui-multi');
 		box.querySelectorAll('input[type="hidden"]').forEach(function (hidden) {
-			if (hidden.name === name) hidden.value = chosen.join('\n');
+			if (hidden.name !== name) return;
+			var order = box.getAttribute('data-ui-multi-order');
+			if (order !== null) {
+				// Capture browser-decoded values once, surviving script reinjection.
+				if (order === '') {
+					order = JSON.stringify(hidden.value.split('\n'));
+					box.setAttribute('data-ui-multi-order', order);
+				}
+				// Retain legacy selection order; newly added entries go first.
+				var checked = new Set(chosen);
+				var previous = (reset ? JSON.parse(order) : hidden.value.split('\n'))
+					.filter(function (value) { return checked.has(value); });
+				var retained = new Set(previous);
+				var added = chosen.filter(function (value) { return !retained.has(value); });
+				hidden.value = added.reverse().concat(previous).join('\n');
+			} else {
+				hidden.value = chosen.join('\n');
+			}
 		});
 	}
 	// Bulk actions affect only visible, enabled rows.
@@ -274,7 +291,7 @@
 			document.querySelectorAll('[data-ui-multi]').forEach(function (box) {
 				if (e.target.contains(box)) {
 					multiAnchors.delete(box);
-					applyMulti(box);
+					applyMulti(box, true);
 				}
 			});
 		}, 0);