Fix the permission trace, tab contrast, touch targets, labels and no-JS forms

.gitattributes

@@ -13,3 +13,4 @@ phpunit.xml.*     export-ignore
 tests             export-ignore
 docs              export-ignore
 .editorconfig     export-ignore
+.gitignore        export-ignore

adm/style/acp_knowledgebase_config.html

@@ -81,12 +81,10 @@ <h1>{{ lang('ACP_KNOWLEDGE_BASE_CONFIGURE') }}</h1>
 				<span>{{ lang('KB_INTRO_ENABLE_EXPLAIN') }}</span>
 			</dt>
 			<dd>
-				<input name="intro_enabled" id="intro_enabled" type="checkbox" value="1"
-					onClick="document.getElementById('intro_options').style.display = this.checked ? '' : 'none';"
-					{{ INTRO_ON }}/>
+				<input name="intro_enabled" id="intro_enabled" type="checkbox" value="1" {{ INTRO_ON }}/>
 			</dd>
 		</dl>
-		<div id="intro_options" style="display: {{ INTRO_DISPLAY }};">
+		<div id="intro_options">
 			<dl>
 				<dt>
 					<label for="intro_text">{{ lang('KB_INTRO_TEXT') }}{{ lang('COLON') }}</label><br>

adm/style/acp_knowledgebase_fields.html

@@ -68,7 +68,7 @@ <h1>{{ lang('ACP_LIBRARY_FIELDS') }}</h1>
 			</dl>
 			<dl>
 				<dt><label for="field_type">{{ lang('KB_FIELD_TYPE') }}{{ lang('COLON') }}</label></dt>
-				<dd><label><select name="field_type" id="field_type">{{ TYPE_OPTIONS }}</select></label></dd>
+				<dd><select name="field_type" id="field_type">{{ TYPE_OPTIONS }}</select></dd>
 			</dl>
 			<dl>
 				<dt><label for="field_required">{{ lang('KB_FIELD_REQUIRED') }}{{ lang('COLON') }}</label><br>

adm/style/js/acp_knowledgebase.js

@@ -10,4 +10,15 @@
 			$icon.attr('class', '');
 		}
 	});
+	// The introduction's text and Markdown options show only while it is
+	// switched on. They are in the page from the start, so the setting still
+	// works without JavaScript.
+	let $introEnabled = $('#intro_enabled');
+	let $introOptions = $('#intro_options');
+	if ($introEnabled.length && $introOptions.length) {
+		$introOptions.toggle($introEnabled.prop('checked'));
+		$introEnabled.on('change', function() {
+			$introOptions.toggle(this.checked);
+		});
+	}
 })(jQuery);

controller/acp_controller.php

@@ -288,7 +288,6 @@ public function settings(): void
 			'ADVANCED_FORM'            => $announce_forum_id ? '' : 'none',
 			'HIERARCHICAL_INDEX_ON'    => $this->config['kb_hierarchical_index'] ? 'checked="checked"' : '',
 			'INTRO_ON'                 => $this->intro && $this->intro->is_enabled() ? 'checked="checked"' : '',
-			'INTRO_DISPLAY'            => $this->intro && $this->intro->is_enabled() ? '' : 'none',
 			'INTRO_TEXT'               => $this->intro ? $this->intro->for_edit() : '',
 			'INTRO_MARKDOWN_ON'        => $this->intro && $this->intro->is_markdown() ? 'checked="checked"' : '',
 			'ALT_STYLE_ON'             => $this->config['kb_alt_style_enabled'] ? 'checked="checked"' : '',
@@ -3067,7 +3066,7 @@ protected function kb_acl_group_raw_data($group_id = false, $opts = false, $cate
 		$sql_ary[] = 'SELECT a.group_id, a.category_id, a.auth_setting, a.auth_option_id, ao.auth_option
 			FROM ' . $this->kb_groups_table . ' a, ' . $this->options_table . ' ao
 			WHERE a.auth_option_id = ao.auth_option_id ' .
-			(($sql_group) ? 'AND a.' . $sql_group : '') .
+			(($sql_group) ? 'AND a.' . $sql_group : '') . ' ' .
 			$sql_category . ' ' .
 			$sql_opts . '
 			ORDER BY a.category_id, ao.auth_option';
@@ -3099,11 +3098,11 @@ protected function build_auth_option_statement($key, $auth_options, &$sql_opts):
 		{
 			if (str_contains($auth_options, '%'))
 			{
-				$sql_opts = 'AND ' . $key . $this->db->sql_like_expression(str_replace('%', $this->db->get_any_char(), $auth_options));
+				$sql_opts = 'AND ' . $key . ' ' . $this->db->sql_like_expression(str_replace('%', $this->db->get_any_char(), $auth_options));
 			}
 			else
 			{
-				$sql_opts = 'AND ' . $key . ' = ' . $this->db->sql_escape($auth_options);
+				$sql_opts = 'AND ' . $key . " = '" . $this->db->sql_escape($auth_options) . "'";
 			}
 		}
 		else
@@ -3162,7 +3161,7 @@ protected function kb_acl_user_raw_data($user_id = false, $opts = false, $catego
 		// Grab user settings - non-role specific...
 		$sql_ary[] = 'SELECT a.user_id, a.category_id, a.auth_setting, a.auth_option_id, ao.auth_option
 			FROM ' . $this->kb_users_table . ' a, ' . $this->options_table . ' ao
-			WHERE a.auth_option_id = ao.auth_option_id ' . (($sql_user) ? 'AND a.' . $sql_user : '') .
+			WHERE a.auth_option_id = ao.auth_option_id ' . (($sql_user) ? 'AND a.' . $sql_user : '') . ' ' .
 			$sql_category . ' ' . $sql_opts . '
 			ORDER BY a.category_id, ao.auth_option';
 

docs/testing.md

@@ -1593,8 +1593,8 @@ dropdown, the copy-link boxes, the article text box, custom-field dropdowns, the
 action menus, and in the ACP the category picker, the new-category box and the file-size unit menu.
 Each is now labelled, and the Teams block of the ACP configuration page uses valid definition-list
 markup. What remains is prosilver's own markup (header menus, footer links, the pagination list,
-plupload's hidden file input), the inactive tabs' contrast, which comes from prosilver's tab colours
-and differs per style, and small touch targets in the article lists and queue rows.
+plupload's hidden file input). The inactive tabs' contrast and the small touch targets in the
+article lists and queue rows were fixed afterwards (next section).
 
 phpbb-release-tools gained a Knowledge Base feature check (`check_knowledgebase` in
 `feature_checks.py`): the ACP adds a category and refuses its form without a token and a move link
@@ -1611,3 +1611,58 @@ the current development build's code (`313eac3`, 40 migrations) on SQLite and My
 to 43 and passed the sweep and the feature check. The PHPUnit suite passes on phpBB 3.3.x and
 master with the MySQL and PostgreSQL fixtures: 578 tests, none skipped. EPV and phpcs are clean on
 both branches.
+
+## Tab contrast, touch targets, labels and forms without JavaScript - 11/12/2026
+
+Follow-ups from the maintenance pass:
+
+- The inactive tabs showed prosilver's grey-blue text (`#536482`) on its tab colour (`#BACCD9`),
+  3.7:1 for small bold text that needs 4.5:1. They now use the board's dark text colour
+  (`#28313F`), about 8.8:1 on the same background. A style with dark tabs can change it in an
+  alternative stylesheet.
+- Article title links in the lists were 13 to 17px high, and the queue's buttons and menus 19 to 21px.
+  All are now at least 24px (WCAG 2.2, 2.5.8).
+- Three fields on the posting form (post as, also list in, Markdown) and the ACP field type menu had
+  two labels: the row's own `label for` and another wrapped around the field. The wrapping one is
+  gone.
+- The ACP introduction settings were hidden by the server and shown only by a script, so they could
+  not be reached without JavaScript. They are now in the page from the start, and the script hides
+  them while the introduction is off. The custom-field filter and the "drafts only" box submit on
+  change; they now get a Go button when JavaScript is off.
+- `.gitignore` is left out of release archives.
+
+`tests/browser/kb_form_access.js` checks each of these on a seeded board, as the administrator,
+with and without JavaScript:
+
+```bash
+KB_TEST_URL=http://localhost:8241 node /path/to/playwright-skill/run.js tests/browser/kb_form_access.js
+```
+
+On the code before the fix, 7 of its 9 checks fail; the two that pass check that the introduction
+settings still follow the switch with JavaScript on. Claude ran it and `kb_page_sweep.js` on fresh
+seed-forum SQLite boards for phpBB 3.3.19 and 4.0.0-a3-dev (9 of 9, and 26 of 26 pages, on both).
+A phpBB 3.3 board seeded by seed-forum turns clean URLs on, which PHP's built-in web server cannot
+route; turn `enable_mod_rewrite` off on such a board before running the browser checks through it.
+
+## The ACP permission trace - 11/12/2026
+
+A validation report from another phpBB developer's tool listed two missing language keys,
+`KB_TRACE_GROUP_NEVER_TOTAL_NEVER` and its `_LOCAL` form: the trace shows them when a group is set
+to NEVER and the total is already NEVER. They are added, worded as phpBB's own trace.
+
+Opening the trace page to test them showed that it could not open at all:
+
+- `build_auth_option_statement()` put a single permission name into SQL without quotes, unlike
+  phpBB's own version of the function. The name comes from the trace page's address, so a crafted
+  link opened by an administrator could run SQL of its choosing. The name is now a quoted string. Its
+  `LIKE` branch also lacked the space before `LIKE`.
+- The group and user queries joined `group_id IN (...)` and `user_id = N` to the next condition
+  without a space (`2AND`).
+
+`tests/kb_acp_auth_option_statement_test.php` runs the option filter against a SQLite table: a
+single name, a name with SQL in it, a `LIKE` pattern and a list. Three of its four tests fail on
+the code before the fix. `tests/browser/kb_page_sweep.js` now also opens the trace for the
+administrator in the first category, which fails with an SQL error on the old code. On the fixed code,
+with two of the administrator's groups set to NEVER, the trace shows every row in words. Claude ran
+the sweep (27 of 27) and `kb_form_access.js` (9 of 9) on the phpBB 3.3.19 and 4.0.0-a3-dev SQLite
+boards.

language/en/acp_knowledgebase.php

@@ -225,6 +225,7 @@
 'LOG_LIBRARY_PERMISSION_ADD'                 => 'Adding or changing user/group access to category <strong>%1s</strong><br> %2s',
 'LOG_LIBRARY_CONFIG'                         => '<strong>Reconfigured the Knowledge Base</strong>',
 
+'KB_TRACE_GROUP_NEVER_TOTAL_NEVER_LOCAL' => 'This group’s permission for this category is set to <strong>NEVER</strong> like the total result so the old result is kept.',
 'KB_TRACE_GROUP_NEVER_TOTAL_NO_LOCAL'  => 'This group’s permission for this category is set to <strong>NEVER</strong> which becomes the new total value because it wasn’t set yet (set to <strong>NO</strong>).',
 'KB_TRACE_GROUP_NEVER_TOTAL_YES_LOCAL' => 'This group’s permission for this category is set to <strong>NEVER</strong> which overwrites the total <strong>YES</strong> to a <strong>NEVER</strong> for this user.',
 'KB_TRACE_GROUP_NO_LOCAL'              => 'The permission is <strong>NO</strong> for this group within this category so the old total value is kept.',
@@ -234,6 +235,7 @@
 
 // Same as the _LOCAL versions above, but for the global (non-category) trace view -
 // controller/acp_controller.php picks between the two sets based on whether a category_id is present.
+'KB_TRACE_GROUP_NEVER_TOTAL_NEVER' => 'This group’s permission is set to <strong>NEVER</strong> like the total result so the old result is kept.',
 'KB_TRACE_GROUP_NEVER_TOTAL_NO'  => 'This group’s permission is set to <strong>NEVER</strong> which becomes the new total value because it wasn’t set yet (set to <strong>NO</strong>).',
 'KB_TRACE_GROUP_NEVER_TOTAL_YES' => 'This group’s permission is set to <strong>NEVER</strong> which overwrites the total <strong>YES</strong> to a <strong>NEVER</strong> for this user.',
 'KB_TRACE_GROUP_NO'              => 'The permission is <strong>NO</strong> for this group so the old total value is kept.',

styles/all/template/kb_cat_body.html

@@ -71,6 +71,7 @@ <h2>{{ CATEGORY }}</h2>
 				{{ FIELD_FILTER_OPTIONS }}
 			</select>
 		</label>
+		<noscript><button class="button2" type="submit">{{ lang('GO') }}</button></noscript>
 		{% if S_FIELD_FILTER_ACTIVE %}
 			<a href="{{ U_RESET_FIELD_FILTER }}">{{ lang('RESET_FILTER') }}</a>
 		{% endif %}

styles/all/template/kb_my_articles_body.html

@@ -24,6 +24,7 @@ <h2><a href="{{ U_LIBRARY }}" class="nav">{{ lang('LIBRARY') }}</a> &raquo; {{ l
 
 <form method="get" action="{{ S_ACTION }}" class="action-bar bar-top">
 	<label><input type="checkbox" name="drafts_only" value="1" onchange="this.form.submit();"{% if S_DRAFTS_ONLY %} checked="checked"{% endif %}> {{ lang('KB_DRAFTS_ONLY') }}</label>
+	<noscript><button class="button2" type="submit">{{ lang('GO') }}</button></noscript>
 </form>
 
 <div class="forumbg forumbg-table kb-listing">

styles/all/template/kb_post_body.html

@@ -29,7 +29,7 @@ <h3>{{ lang('POST_A') }}</h3>
 				{% if not S_EDIT_POST and S_CAN_POST_AS_TEAM %}
 					<dl style="clear: left;">
 						<dt><label for="post_as_group">{{ lang('KB_POST_AS') }}{{ lang('COLON') }}</label></dt>
-						<dd><label><select name="post_as_group" id="post_as_group">{{ TEAM_OPTIONS }}</select></label></dd>
+						<dd><select name="post_as_group" id="post_as_group">{{ TEAM_OPTIONS }}</select></dd>
 					</dl>
 				{% endif %}
 				{% if S_CAN_CHOOSE_SUBMIT_AS %}
@@ -66,8 +66,8 @@ <h3>{{ lang('POST_A') }}</h3>
 				<dl style="clear: left;">
 					<dt><label for="cross_categories">{{ lang('KB_CROSS_CATEGORIES') }}{{ lang('COLON') }}</label><br>
 						<span>{{ lang('KB_CROSS_CATEGORIES_EXPLAIN') }}</span></dt>
-					<dd><label><select name="cross_categories[]" id="cross_categories" multiple="multiple"
-								size="6">{{ CROSS_CATS_BOX }}</select></label></dd>
+					<dd><select name="cross_categories[]" id="cross_categories" multiple="multiple"
+								size="6">{{ CROSS_CATS_BOX }}</select></dd>
 				</dl>
 				{{ FIELD_WIDGETS }}
 				<dl style="clear: left;">
@@ -92,7 +92,7 @@ <h3>{{ lang('POST_A') }}</h3>
 					<dl style="clear: left;">
 						<dt><label for="article_markdown">{{ lang('KB_USE_MARKDOWN') }}{{ lang('COLON') }}</label><br>
 							<span>{{ lang('KB_USE_MARKDOWN_EXPLAIN') }}</span></dt>
-						<dd><label><input type="checkbox" name="article_markdown" id="article_markdown" value="1"{% if S_ARTICLE_MARKDOWN %} checked="checked"{% endif %}/> {{ lang('YES') }}</label>
+						<dd><input type="checkbox" name="article_markdown" id="article_markdown" value="1"{% if S_ARTICLE_MARKDOWN %} checked="checked"{% endif %}/> {{ lang('YES') }}
 							<input type="hidden" name="article_markdown_form" value="1"/></dd>
 					</dl>
 					{% INCLUDEJS '@phpbbmodders_knowledgebase/js/markdown_toggle.js' %}

styles/all/theme/phpbbmodders_knowledgebase.css

@@ -291,6 +291,25 @@ table.kb-index-table .kb-index-author {
 	overflow-wrap: break-word;
 }
 
+/*
+ * Touch targets at least 24px high (WCAG 2.2, 2.5.8): the article title links
+ * and the queue's buttons and menus sat closer together than that on a phone.
+ */
+table.kb-index-table td:first-child > a {
+	display: inline-block;
+	min-height: 24px;
+	line-height: 24px;
+}
+
+table.kb-index-table .button2,
+table.kb-index-table select {
+	display: inline-block;
+	box-sizing: border-box;
+	min-height: 24px;
+	min-width: 24px;
+	vertical-align: middle;
+}
+
 /*
  * Headings in article text. Markdown articles can use all six levels, which
  * a forum post never does: prosilver paints an h1 white and large because it
@@ -366,6 +385,17 @@ table.kb-index-table .kb-index-author {
 	font-size: 0.95em;
 }
 
+/*
+ * prosilver's grey-blue tab text is too faint for these small labels on its
+ * tab colour (3.7:1). The board's dark text colour keeps the style's tab
+ * background and passes WCAG AA (4.5:1). An alternative stylesheet can change
+ * it for a style with dark tabs.
+ */
+.tabs.kb-tabs .kb-tab:not(.kb-tab-active) > a,
+.tabs.kb-tabs .kb-tab:not(.kb-tab-active) > a:hover {
+	color: #28313f;
+}
+
 .tabs.kb-tabs .kb-tab-active > a,
 .tabs.kb-tabs .kb-tab-active > a:hover {
 	background: transparent;