jody/pl_mail
1
0 Fork 0
mirror of https://github.com/karatektus/pl_mail.git synced 2026-10-25 10:04:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(search,routing,a11y): prefix+substring search, page clamps, 404s, contrast

Browse Source 
H-04 search found only exact whole words. The tsvector tokenizer emits
"help.wirhub.de" as one host lexeme, so "wirhub" is a substring of a lexeme and
no full-text query can reach it, while "Testmai" needs a prefix operator
websearch_to_tsquery has no way to express. Free text now compiles to three
OR-ed passes — websearch as before, a to_tsquery with :* per term, and an ILIKE
needle for single terms of 3+ characters — with pg_trgm GIN indexes so the third
is index-backed. Deliberate websearch syntax (quotes, OR, negation) suppresses
the extra passes rather than widening what was narrowed on purpose.

H-06 page numbers past the end rendered "201-193 of 193" over an empty list.
The six role views were six copies of the same eight lines, so they are now one.

H-07 /mail/thread/abc and ?tab=quatsch were 500s: a non-numeric id reaching the
entity resolver, and MessageCategory::from on a URL parameter. Error pages are
branded and localised, and deliberately standalone — the app shell asks for a
session, which an anonymous 404 does not have.

A-01/A-02 <html> had no lang at all, and the viewport forbade zoom.

A-07 accent ink was effectively always white (2.34:1 on the reported pink); it
is now chosen by WCAG relative luminance on gamma-expanded channels, scoring
both candidates instead of guessing at a threshold. Every theme's ink ramp now
clears 4.5:1 on its own surface and still runs strongest-to-faintest.

I-02 search.title existed in no catalogue.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Claude committed 2026-08-11 17:09:13 +02:00
1 parent 7fc7f88598
commit 5ff870c3b7
29 files changed
+1348 -139
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

assets/styles/app.css
+31 -26
View File
@@ -55,8 +55,8 @@
/* Ink */
--rgb-ink: 39 39 42;
--rgb-ink-soft: 55 65 81; /* gray-700 */
--rgb-ink-muted: 113 113 122;
--rgb-ink-faint: 161 161 170;
--rgb-ink-muted: 95 95 102;
--rgb-ink-faint: 116 116 122;
/* Accent. The stored setting overrules this from an inline style on
<html>; the value here is what the theme asks for, and Theme::defaults()
@@ -108,8 +108,8 @@
--rgb-sheet: 255 255 255;
--rgb-sheet-ink: 39 39 42;
--rgb-sheet-ink-soft: 55 65 81;
--rgb-sheet-ink-muted: 113 113 122;
--rgb-sheet-ink-faint: 161 161 170;
--rgb-sheet-ink-muted: 95 95 102;
--rgb-sheet-ink-faint: 116 116 122;
--rgb-sheet-link: 29 78 216;
--rgb-sheet-danger: 185 28 28;
@@ -147,7 +147,7 @@
--rgb-ink: 244 244 245;
--rgb-ink-soft: 209 213 219; /* gray-300 */
--rgb-ink-muted: 161 161 170;
--rgb-ink-faint: 113 113 122;
--rgb-ink-faint: 129 129 137;
--rgb-accent: 37 99 235;
--rgb-accent-ink: 255 255 255;
@@ -185,8 +185,8 @@
--rgb-sheet: 248 250 252; /* slate-50 */
--rgb-sheet-ink: 15 23 42; /* slate-900 */
--rgb-sheet-ink-soft: 51 65 85; /* slate-700 */
--rgb-sheet-ink-muted: 100 116 139; /* slate-500 */
--rgb-sheet-ink-faint: 148 163 184; /* slate-400 */
--rgb-sheet-ink-muted: 81 94 113; /* slate-500 */
--rgb-sheet-ink-faint: 104 114 129; /* slate-400 */
--rgb-sheet-link: 29 78 216;
--rgb-sheet-danger: 185 28 28;
}
@@ -215,8 +215,8 @@
--rgb-ink: 35 34 32;
--rgb-ink-soft: 82 78 72;
--rgb-ink-muted: 107 102 94;
--rgb-ink-faint: 150 144 134;
--rgb-ink-muted: 95 91 84;
--rgb-ink-faint: 116 111 103;
--rgb-accent: 125 107 79;
--rgb-accent-ink: 255 255 255;
@@ -255,8 +255,8 @@
--rgb-sheet: 247 245 239;
--rgb-sheet-ink: 35 34 32;
--rgb-sheet-ink-soft: 82 78 72;
--rgb-sheet-ink-muted: 107 102 94;
--rgb-sheet-ink-faint: 150 144 134;
--rgb-sheet-ink-muted: 95 91 84;
--rgb-sheet-ink-faint: 116 111 103;
--rgb-sheet-link: 29 78 216;
--rgb-sheet-danger: 185 28 28;
}
@@ -292,8 +292,8 @@
--rgb-ink: 214 209 202;
--rgb-ink-soft: 186 181 174;
--rgb-ink-muted: 142 137 130;
--rgb-ink-faint: 108 104 98;
--rgb-ink-muted: 159 155 149;
--rgb-ink-faint: 134 131 126;
--rgb-accent: 58 111 92;
--rgb-accent-ink: 240 245 242;
@@ -338,8 +338,8 @@
--rgb-sheet: 248 250 252; /* slate-50 */
--rgb-sheet-ink: 15 23 42; /* slate-900 */
--rgb-sheet-ink-soft: 51 65 85; /* slate-700 */
--rgb-sheet-ink-muted: 100 116 139; /* slate-500 */
--rgb-sheet-ink-faint: 148 163 184; /* slate-400 */
--rgb-sheet-ink-muted: 81 94 113; /* slate-500 */
--rgb-sheet-ink-faint: 104 114 129; /* slate-400 */
--rgb-sheet-link: 29 78 216;
--rgb-sheet-danger: 185 28 28;
}
@@ -395,9 +395,9 @@
lightest colour rather than a warm cream borrowed from elsewhere. */
--rgb-sheet: 236 239 244;
--rgb-sheet-ink: 46 52 64;
--rgb-sheet-ink-soft: 88 93 104;
--rgb-sheet-ink-muted: 111 116 125;
--rgb-sheet-ink-faint: 149 153 161;
--rgb-sheet-ink-soft: 66 70 78;
--rgb-sheet-ink-muted: 83 87 94;
--rgb-sheet-ink-faint: 104 107 113;
--rgb-sheet-link: 94 129 172; /* #5e81ac */
--rgb-sheet-danger: 191 97 106; /* #bf616a */
}
@@ -451,8 +451,8 @@
--rgb-sheet: 245 243 255;
--rgb-sheet-ink: 30 27 46;
--rgb-sheet-ink-soft: 77 75 92;
--rgb-sheet-ink-muted: 103 100 117;
--rgb-sheet-ink-faint: 146 144 159;
--rgb-sheet-ink-muted: 91 88 103;
--rgb-sheet-ink-faint: 111 109 121;
--rgb-sheet-link: 109 40 217;
--rgb-sheet-danger: 185 28 28;
}
@@ -472,10 +472,15 @@
--rgb-sunken: 238 232 213; /* base2 #eee8d5 */
--rgb-shadow: 0 0 0;
--rgb-ink: 88 110 117; /* base01 #586e75 */
--rgb-ink-soft: 94 116 124;
--rgb-ink-muted: 101 123 131; /* base00 #657b83 */
--rgb-ink-faint: 147 161 161; /* base1 #93a1a1 */
/* Solarized light's own content tones (base01/base00/base1) sit inside a
narrow band — roughly 2.5:1 to 5:1 on base3. That band cannot hold four
tiers where even the FAINTEST must clear 4.5:1, so the ramp is extended
downward past base02 rather than compressed into illegibility. The hue
is solarized's; the range is not. */
--rgb-ink: 46 57 61;
--rgb-ink-soft: 61 75 81;
--rgb-ink-muted: 77 93 100; /* base00 #657b83, darkened */
--rgb-ink-faint: 104 114 114;
--rgb-accent: 181 137 0; /* yellow #b58900 */
--rgb-accent-ink: 255 255 255;
@@ -507,8 +512,8 @@
--rgb-sheet: 253 246 227; /* base3 */
--rgb-sheet-ink: 7 54 66; /* base02 #073642 */
--rgb-sheet-ink-soft: 61 96 101;
--rgb-sheet-ink-muted: 91 119 121;
--rgb-sheet-ink-faint: 140 158 153;
--rgb-sheet-ink-muted: 74 96 98;
--rgb-sheet-ink-faint: 102 115 112;
--rgb-sheet-link: 38 139 210; /* blue #268bd2 */
--rgb-sheet-danger: 220 50 47; /* red #dc322f */
}
migrations/Version20260811160000.php
+73
View File
@@ -0,0 +1,73 @@
<?php
declare(strict_types=1);
namespace DoctrineMigrations;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;
/**
* Trigram indexes for the substring pass in search.
*
* Why search needs a substring pass at all is in FreeTextCompiler: the
* tokenizer emits "help.wirhub.de" as one `host` lexeme, so "wirhub" is not a
* prefix of anything in the tsvector and full-text search cannot reach it by
* any query. ILIKE '%wirhub%' can, and without an index that is a sequential
* scan over every body in the account.
*
* pg_trgm ships with the postgres image (available 1.6, not previously
* installed). GIN over gin_trgm_ops lets the planner serve `%needle%` from the
* index for needles of three characters or more, which is exactly the floor
* FreeTextCompiler enforces before it emits a needle at all.
*
* The cost, stated plainly because it is not free:
*
* Write amplification. Every INSERT of a message now maintains four more GIN
* indexes, body_text's being much the largest — a trigram index over prose is
* roughly the size of the prose. Sync is a bulk insert path, so this is the
* number that matters; GIN's pending-list (fastupdate, on by default) absorbs
* it in batches rather than per row.
*
* Disk. Expect the trigram index on body_text to approach the size of the
* body_text column itself. This is the real price of the feature.
*
* The alternative — leaving the substring pass unindexed — was rejected because
* a search box that is correct and slow gets used and then reported as slow,
* whereas one that is fast and wrong gets reported as broken, which is what
* happened.
*
* CONCURRENTLY is deliberately NOT used: it cannot run inside the transaction
* migrations execute in. On an install large enough for that to matter, the
* index build takes a write lock on `message` for its duration.
*/
final class Version20260811160000 extends AbstractMigration
{
public function getDescription(): string
{
return 'Install pg_trgm and add GIN trigram indexes for substring search on message';
}
public function up(Schema $schema): void
{
$this->addSql('CREATE EXTENSION IF NOT EXISTS pg_trgm');
$this->addSql('CREATE INDEX IF NOT EXISTS idx_message_subject_trgm ON message USING GIN (subject gin_trgm_ops)');
$this->addSql('CREATE INDEX IF NOT EXISTS idx_message_body_text_trgm ON message USING GIN (body_text gin_trgm_ops)');
$this->addSql('CREATE INDEX IF NOT EXISTS idx_message_from_name_trgm ON message USING GIN (from_name gin_trgm_ops)');
$this->addSql('CREATE INDEX IF NOT EXISTS idx_message_from_address_trgm ON message USING GIN (from_address gin_trgm_ops)');
}
public function down(Schema $schema): void
{
$this->addSql('DROP INDEX IF EXISTS idx_message_from_address_trgm');
$this->addSql('DROP INDEX IF EXISTS idx_message_from_name_trgm');
$this->addSql('DROP INDEX IF EXISTS idx_message_body_text_trgm');
$this->addSql('DROP INDEX IF EXISTS idx_message_subject_trgm');
// The extension stays. btree_gist is installed by the calendar
// migration and nothing here can know whether something else has since
// come to depend on pg_trgm; dropping a shared extension on the way
// down is how an unrelated index disappears.
}
}
src/Controller/Mail/AttachmentController.php
+2 -2
View File
@@ -31,7 +31,7 @@ final class AttachmentController extends AbstractController
* answers 404 and lets the chip keep its icon, rather than sending the
* full-size original under a preview's name.
*/
#[Route('/mail/attachment/{id}/thumbnail', name: 'app_mail_attachment_thumbnail', methods: ['GET'])]
#[Route('/mail/attachment/{id}/thumbnail', name: 'app_mail_attachment_thumbnail', methods: ['GET'], requirements: ['id' => '\d+'])]
public function thumbnail(MessagePart $part): Response
{
$this->assertOwned($part);
@@ -54,7 +54,7 @@ final class AttachmentController extends AbstractController
return $response;
}
#[Route('/mail/attachment/{id}', name: 'app_mail_attachment', methods: ['GET'])]
#[Route('/mail/attachment/{id}', name: 'app_mail_attachment', methods: ['GET'], requirements: ['id' => '\d+'])]
public function serve(MessagePart $part, Request $request): Response
{
$this->assertOwned($part);
src/Controller/Mail/MailController.php
+123 -91
View File
@@ -18,6 +18,7 @@ use App\Twig\SidebarCounts;
use Doctrine\ORM\EntityManagerInterface;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Routing\Attribute\Route;
@@ -35,21 +36,73 @@ final class MailController extends AbstractController
{
}
/** Rows per page, for every list view and the pagination that clamps them. */
private const int PER_PAGE = 50;
/**
* The page to render — or a redirect to the last page that exists.
*
* `?page=999` on a 193-row folder used to render the header "201–193 of
* 193" above nothing at all: the lower bound was clamped and the upper one
* was not, so the arithmetic ran off the end of the list and said so. The
* page number is user input via the URL, and the honest answer to a page
* that does not exist is the last one that does.
*
* A redirect rather than a silent clamp, so the address bar stops claiming
* a page the user is not on — otherwise reloading, bookmarking or sharing
* the URL all carry the phantom page number along.
*
* Built from the current path and query rather than from a route name: this
* serves ten list actions whose route parameters differ, and the one thing
* they share is that only `page` needs changing.
*/
private function pageOrRedirect(Request $request, int $total): int|RedirectResponse
{
$page = max(1, (int) $request->query->get('page', 1));
$lastPage = max(1, (int) ceil($total / self::PER_PAGE));
if ($page <= $lastPage) {
return $page;
}
$query = $request->query->all();
// Page 1 is the bare URL. Carrying `?page=1` would make the canonical
// first page of every list a different URL depending on how it was
// reached.
if (1 === $lastPage) {
unset($query['page']);
} else {
$query['page'] = $lastPage;
}
return $this->redirect(
$request->getPathInfo() . ([] === $query ? '' : '?' . http_build_query($query)),
);
}
#[Route('/inbox', name: 'inbox')]
public function inbox(
Request $request,
): Response {
$user = $this->getUser();
$tab = MessageCategory::Primary;
$page = max(1, (int) $request->query->get('page', 1));
$tabParam = $request->query->get('tab');
if ($tabParam !== null) {
$tab = MessageCategory::from($tabParam);
// tryFrom, not from: `?tab=` is a URL anyone can edit, and a category
// that does not exist is a mistyped or stale link, not a server fault.
// ValueError out of an enum here surfaced as a 500 on ?tab=quatsch.
// Falling back to Primary shows the inbox, which is what the person
// following that link was after.
$tab = MessageCategory::tryFrom((string) $request->query->get('tab', ''))
?? MessageCategory::Primary;
$total = $this->threadRepository->countForUnifiedInbox($user, $tab);
$page = $this->pageOrRedirect($request, $total);
if ($page instanceof RedirectResponse) {
return $page;
}
$threads = $this->threadRepository->findForUnifiedInbox($user, $tab, $page);
$total = $this->threadRepository->countForUnifiedInbox($user, $tab);
$tabCounts = $this->threadRepository->countUnreadByCategoryForUnifiedInbox($user);
$tabTotals = $this->threadRepository->countByCategoryForUnifiedInbox($user);
@@ -74,7 +127,7 @@ final class MailController extends AbstractController
'tabCounts' => $tabCounts,
'page' => $page,
'total' => $total,
'per_page' => 50,
'per_page' => self::PER_PAGE,
]);
}
@@ -99,7 +152,7 @@ final class MailController extends AbstractController
return $this->renderLabel($label, $request);
}
#[Route('/label/{id}', name: 'label')]
#[Route('/label/{id}', name: 'label', requirements: ['id' => '\d+'])]
public function labelView(Label $label, Request $request): Response
{
if ($label->usr !== $this->getUser()) {
@@ -112,9 +165,14 @@ final class MailController extends AbstractController
private function renderLabel(Label $label, Request $request): Response
{
$account = $this->requestedAccount($request);
$page = max(1, (int) $request->query->get('page', 1));
$threads = $this->threadRepository->findForLabel($label, $account, $page);
$total = $this->threadRepository->countForLabel($label, $account);
$page = $this->pageOrRedirect($request, $total);
if ($page instanceof RedirectResponse) {
return $page;
}
$threads = $this->threadRepository->findForLabel($label, $account, $page);
$this->threadRepository->preloadLabels($threads);
@@ -124,7 +182,7 @@ final class MailController extends AbstractController
'threads' => $threads,
'page' => $page,
'total' => $total,
'per_page' => 50,
'per_page' => self::PER_PAGE,
]);
}
@@ -160,16 +218,21 @@ final class MailController extends AbstractController
* Everything in one account — what clicking the account itself in the
* sidebar now does. Its labels sit underneath for narrowing further.
*/
#[Route('/account/{account}', name: 'account')]
#[Route('/account/{account}', name: 'account', requirements: ['account' => '\d+'])]
public function accountView(Account $account, Request $request): Response
{
if ($account->usr !== $this->getUser()) {
throw $this->createAccessDeniedException();
}
$page = max(1, (int) $request->query->get('page', 1));
$threads = $this->threadRepository->findForAccount($account, $page);
$total = $this->threadRepository->countForAccount($account);
$page = $this->pageOrRedirect($request, $total);
if ($page instanceof RedirectResponse) {
return $page;
}
$threads = $this->threadRepository->findForAccount($account, $page);
$this->threadRepository->preloadLabels($threads);
@@ -178,7 +241,7 @@ final class MailController extends AbstractController
'threads' => $threads,
'page' => $page,
'total' => $total,
'per_page' => 50,
'per_page' => self::PER_PAGE,
]);
}
@@ -186,9 +249,14 @@ final class MailController extends AbstractController
public function starred(Request $request): Response
{
$user = $this->getUser();
$page = max(1, (int) $request->query->get('page', 1));
$threads = $this->threadRepository->findForStarred($user, $page);
$total = $this->threadRepository->countForStarred($user);
$page = $this->pageOrRedirect($request, $total);
if ($page instanceof RedirectResponse) {
return $page;
}
$threads = $this->threadRepository->findForStarred($user, $page);
$this->threadRepository->preloadLabels($threads);
@@ -196,62 +264,26 @@ final class MailController extends AbstractController
'threads' => $threads,
'page' => $page,
'total' => $total,
'per_page' => 50,
'per_page' => self::PER_PAGE,
]);
}
#[Route('/sent', name: 'sent')]
public function sent(Request $request): Response
{
$user = $this->getUser();
$page = max(1, (int) $request->query->get('page', 1));
$threads = $this->threadRepository->findForRole($user, LabelRole::Sent, $page);
$total = $this->threadRepository->countForRole($user, LabelRole::Sent);
$this->threadRepository->preloadLabels($threads);
return $this->render('mail/sent.html.twig', [
'threads' => $threads,
'page' => $page,
'total' => $total,
'per_page' => 50,
]);
return $this->renderRole($request, LabelRole::Sent, 'mail/sent.html.twig');
}
#[Route('/drafts', name: 'drafts')]
public function drafts(Request $request): Response
{
$user = $this->getUser();
$page = max(1, (int) $request->query->get('page', 1));
$threads = $this->threadRepository->findForRole($user, LabelRole::Drafts, $page);
$total = $this->threadRepository->countForRole($user, LabelRole::Drafts);
$this->threadRepository->preloadLabels($threads);
return $this->render('mail/drafts.html.twig', [
'threads' => $threads,
'page' => $page,
'total' => $total,
'per_page' => 50,
]);
return $this->renderRole($request, LabelRole::Drafts, 'mail/drafts.html.twig');
}
#[Route('/trash', name: 'trash')]
public function trash(Request $request): Response
{
$user = $this->getUser();
$page = max(1, (int) $request->query->get('page', 1));
$threads = $this->threadRepository->findForRole($user, LabelRole::Trash, $page);
$total = $this->threadRepository->countForRole($user, LabelRole::Trash);
$this->threadRepository->preloadLabels($threads);
return $this->render('mail/trash.html.twig', [
'threads' => $threads,
'page' => $page,
'total' => $total,
'per_page' => 50,
]);
return $this->renderRole($request, LabelRole::Trash, 'mail/trash.html.twig');
}
/**
@@ -267,38 +299,14 @@ final class MailController extends AbstractController
#[Route('/spam', name: 'spam')]
public function spam(Request $request): Response
{
$user = $this->getUser();
$page = max(1, (int) $request->query->get('page', 1));
$threads = $this->threadRepository->findForRole($user, LabelRole::Spam, $page);
$total = $this->threadRepository->countForRole($user, LabelRole::Spam);
$this->threadRepository->preloadLabels($threads);
return $this->render('mail/spam.html.twig', [
'threads' => $threads,
'page' => $page,
'total' => $total,
'per_page' => 50,
]);
return $this->renderRole($request, LabelRole::Spam, 'mail/spam.html.twig');
}
/** Archive role view. */
#[Route('/archive', name: 'archive')]
public function archive(Request $request): Response
{
$user = $this->getUser();
$page = max(1, (int) $request->query->get('page', 1));
$threads = $this->threadRepository->findForRole($user, LabelRole::Archive, $page);
$total = $this->threadRepository->countForRole($user, LabelRole::Archive);
$this->threadRepository->preloadLabels($threads);
return $this->render('mail/archive.html.twig', [
'threads' => $threads,
'page' => $page,
'total' => $total,
'per_page' => 50,
]);
return $this->renderRole($request, LabelRole::Archive, 'mail/archive.html.twig');
}
/**
@@ -312,23 +320,41 @@ final class MailController extends AbstractController
*/
#[Route('/snoozed', name: 'snoozed')]
public function snoozed(Request $request): Response
{
return $this->renderRole($request, LabelRole::Snoozed, 'mail/snoozed.html.twig');
}
/**
* The six role views — Sent, Drafts, Trash, Spam, Archive, Snoozed — differ
* only in which role they count and which template they hand it to.
*
* They were six copies of the same eight lines until the page clamp had to
* be added to all of them, which is the usual way a fix reaches five of six
* places. One body means the next such change lands everywhere or nowhere.
*/
private function renderRole(Request $request, LabelRole $role, string $template): Response
{
$user = $this->getUser();
$page = max(1, (int) $request->query->get('page', 1));
$threads = $this->threadRepository->findForRole($user, LabelRole::Snoozed, $page);
$total = $this->threadRepository->countForRole($user, LabelRole::Snoozed);
$total = $this->threadRepository->countForRole($user, $role);
$page = $this->pageOrRedirect($request, $total);
if ($page instanceof RedirectResponse) {
return $page;
}
$threads = $this->threadRepository->findForRole($user, $role, $page);
$this->threadRepository->preloadLabels($threads);
return $this->render('mail/snoozed.html.twig', [
return $this->render($template, [
'threads' => $threads,
'page' => $page,
'total' => $total,
'per_page' => 50,
'per_page' => self::PER_PAGE,
]);
}
#[Route('/account/{account}/folders', name: 'account_folders')]
#[Route('/account/{account}/folders', name: 'account_folders', requirements: ['account' => '\d+'])]
public function accountFolders(Account $account): Response
{
$this->denyAccessUnlessGranted('ROLE_USER');
@@ -434,7 +460,7 @@ final class MailController extends AbstractController
return $this->json($payload);
}
#[Route('/message/{id}', name: 'message')]
#[Route('/message/{id}', name: 'message', requirements: ['id' => '\d+'])]
public function message(Message $message, Request $request): Response
{
$this->denyAccessUnlessGranted('ROLE_USER');
@@ -463,7 +489,13 @@ final class MailController extends AbstractController
}
#[Route('/thread/{id}', name: 'thread')]
/**
* `{id}` is constrained to digits so a non-numeric id fails to MATCH the
* route and becomes a 404. Without it the id reaches the entity resolver,
* which asks Postgres for a MessageThread whose bigint id is 'abc' and gets
* back a driver-level type error — a 500 for what is only a bad link.
*/
#[Route('/thread/{id}', name: 'thread', requirements: ['id' => '\d+'])]
public function thread(MessageThread $thread, Request $request): Response
{
$this->denyAccessUnlessGranted('ROLE_USER');
src/Controller/Mail/MessageSourceController.php
+1 -1
View File
@@ -18,7 +18,7 @@ use Symfony\Component\Security\Http\Attribute\IsGranted;
* No raw RFC822 blob is stored, so the source view is reconstructed from the
* persisted header map plus the decoded body.
*/
#[Route('/mail/message/{id}', name: 'app_mail_message_')]
#[Route('/mail/message/{id}', name: 'app_mail_message_', requirements: ['id' => '\d+'])]
#[IsGranted('ROLE_USER')]
final class MessageSourceController extends AbstractController
{
src/Repository/Mail/MessageThreadRepository.php
+52 -6
View File
@@ -9,6 +9,7 @@ use App\Domain\Enum\Mail\SearchSortOrder;
use App\Entity\Mail\Account;
use App\Entity\Label\Label;
use App\Entity\Mail\MessageThread;
use App\Service\Search\FreeTextCompiler;
use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository;
use Doctrine\DBAL\ArrayParameterType;
use Doctrine\DBAL\ParameterType;
@@ -18,8 +19,10 @@ use Symfony\Component\Security\Core\User\UserInterface;
class MessageThreadRepository extends ServiceEntityRepository
{
public function __construct(ManagerRegistry $registry)
{
public function __construct(
ManagerRegistry $registry,
private readonly FreeTextCompiler $freeText = new FreeTextCompiler(),
) {
parent::__construct($registry, MessageThread::class);
}
@@ -680,13 +683,56 @@ class MessageThreadRepository extends ServiceEntityRepository
$params['userId'] = $user->id;
$types['userId'] = ParameterType::INTEGER;
// ── Free-text via tsvector ────────────────────────────────────────
// ── Free-text: three passes, OR-ed ────────────────────────────────
// websearch_to_tsquery alone answers only the question "does this mail
// contain exactly these words", which is not the question anyone types
// into a search box. The other two passes are additive — nothing that
// matched before stops matching — and each costs something:
//
// websearch index-only, GIN on search_vector. Free.
// prefix the same GIN index; `:*` makes it a range over the
// lexeme dictionary rather than a point lookup, so it
// reads more index entries but no more heap.
// substring ILIKE '%needle%'. Sequential without help, which is why
// Version20260811... installs pg_trgm and GIN trigram
// indexes over the four columns it touches. Restricted to
// a single term of 3+ characters — see FreeTextCompiler,
// which is also where the safety of the tsquery text is
// established.
$rankExpr = '0';
if ($query->freeText !== '') {
$where[] = "m.search_vector @@ websearch_to_tsquery('english', :freeText)";
$params['freeText'] = $query->freeText;
$rankExpr = "ts_rank(m.search_vector, websearch_to_tsquery('english', :freeText))";
$free = $this->freeText->compile($query->freeText);
$matches = ["m.search_vector @@ websearch_to_tsquery('english', :freeText)"];
$ranks = ["ts_rank(m.search_vector, websearch_to_tsquery('english', :freeText))"];
$params['freeText'] = $free->websearch;
if (null !== $free->prefix) {
$matches[] = "m.search_vector @@ to_tsquery('english', :freePrefix)";
$ranks[] = "ts_rank(m.search_vector, to_tsquery('english', :freePrefix))";
$params['freePrefix'] = $free->prefix;
}
if (null !== $free->substring) {
// from_name and from_address as well as the body: a search for
// part of a sender's domain is the same gesture as a search for
// part of a word in the text, and fails the same way without it.
$matches[] = '('
. 'm.subject ILIKE :freeLike'
. ' OR m.body_text ILIKE :freeLike'
. ' OR m.from_name ILIKE :freeLike'
. ' OR m.from_address ILIKE :freeLike'
. ')';
$params['freeLike'] = $this->freeText->likePattern($free->substring);
}
$where[] = '(' . implode(' OR ', $matches) . ')';
// A row reached only by the substring pass has no ts_rank to give;
// GREATEST over what did match keeps relevance ordering meaningful
// instead of letting a NULL sink every such row to the bottom.
$rankExpr = 1 === count($ranks) ? $ranks[0] : 'GREATEST(' . implode(', ', $ranks) . ')';
}
// ── Operator filters ──────────────────────────────────────────────
src/Service/Appearance/AppearanceRenderer.php
+67 -3
View File
@@ -9,6 +9,9 @@ use App\Entity\Embeddable\Appearance;
final readonly class AppearanceRenderer
{
/** WCAG 2.x AA for body text. What accent-ink is held to. */
private const float MIN_CONTRAST = 4.5;
public function __construct(private BackgroundResolver $backgroundResolver)
{
}
@@ -103,13 +106,74 @@ final readonly class AppearanceRenderer
);
}
/**
* The ink to write ON the accent, chosen by measuring rather than assuming.
*
* The accent is picked by the user from a colour wheel, so no fixed ink can
* be right for all of it — and white was effectively the fixed answer: the
* old test weighted the raw 0–255 channels and only crossed its 0.6
* threshold for very light colours, so a mid-tone pink got white text at
* 2.34:1, well under the 4.5:1 WCAG demands of body text.
*
* Two corrections. First, luminance is computed the way WCAG defines it —
* on gamma-EXPANDED channels. sRGB is not linear in intensity, and skipping
* the expansion overstates how dark a saturated mid-tone is, which is
* exactly the region where the wrong ink gets chosen. Second, there is no
* threshold to tune: both candidate inks are scored against the accent and
* the better one wins, so the rule states its own intent and cannot drift.
*
* The dark candidate is normally #18181B, which is what dark text is
* everywhere else in the app; a different black for accent chips alone
* would read as a mistake. But two inks are not quite enough to promise
* 4.5:1: the accents where white and #18181B score equally sit at a
* relative luminance near 0.198, and both score 4.24:1 there. That band is
* narrow and it is real, so when the house ink cannot clear 4.5:1 the dark
* candidate becomes pure black, which lifts the worst case over the line
* (4.58:1) for every accent a user can pick. In practice this only engages
* for mid-tone accents; everything else still gets the house ink.
*/
private static function contrastChannels(string $hex): string
{
[$r, $g, $b] = self::rgb($hex);
$accent = self::relativeLuminance(self::rgb($hex));
$luminance = (0.2126 * $r + 0.7152 * $g + 0.0722 * $b) / 255;
$houseInk = [24, 24, 27];
$lightInk = [255, 255, 255];
$blackInk = [0, 0, 0];
return $luminance > 0.6 ? '24 24 27' : '255 255 255';
$onHouse = self::contrastRatio($accent, self::relativeLuminance($houseInk));
$onLight = self::contrastRatio($accent, self::relativeLuminance($lightInk));
if ($onLight > $onHouse && $onLight > self::contrastRatio($accent, self::relativeLuminance($blackInk))) {
return sprintf('%d %d %d', ...$lightInk);
}
$ink = $onHouse >= self::MIN_CONTRAST ? $houseInk : $blackInk;
return sprintf('%d %d %d', ...$ink);
}
/**
* WCAG 2.x relative luminance.
*
* @param array{0:int,1:int,2:int} $rgb
*/
private static function relativeLuminance(array $rgb): float
{
$expand = static function (int $channel): float {
$c = $channel / 255;
return $c <= 0.04045 ? $c / 12.92 : (($c + 0.055) / 1.055) ** 2.4;
};
return 0.2126 * $expand($rgb[0])
+ 0.7152 * $expand($rgb[1])
+ 0.0722 * $expand($rgb[2]);
}
/** WCAG contrast ratio, 1.0 (identical) to 21.0 (black on white). */
private static function contrastRatio(float $a, float $b): float
{
return (max($a, $b) + 0.05) / (min($a, $b) + 0.05);
}
/** @return array{0:int,1:int,2:int} */
src/Service/Search/FreeTextCompiler.php
+123
View File
@@ -0,0 +1,123 @@
<?php
declare(strict_types=1);
namespace App\Service\Search;
/**
* Turns the free-text remainder of a search into the forms Postgres can match.
*
* Why this is not just websearch_to_tsquery
* -----------------------------------------
* A tsvector holds LEXEMES, and a lexeme only matches whole. Two consequences
* bit users hard enough to be reported as one bug:
*
* 1. "Testmai" finds nothing, because the mail contains "Testmail" and a
* tsquery without `:*` asks for equality, not for a beginning.
* 2. "wirhub" finds nothing in a body that plainly reads "help.wirhub.de",
* because the tokenizer classifies that as ONE `host` token. No amount of
* prefix matching fixes this one — "wirhub" is not a prefix of
* "help.wirhub.de", it is in the middle of it — so it needs a substring
* match, which full-text search structurally cannot offer.
*
* So: prefix for (1), a needle for (2), and the original string for everything
* websearch already got right.
*
* Safety
* ------
* to_tsquery parses its argument as an expression, so user text cannot be
* pasted into it — `a:*&!b` typed in the box would otherwise be executable
* syntax, and an unbalanced quote a 500. Every term is stripped to characters
* that cannot mean anything to the tsquery parser and then wrapped in single
* quotes, which is the one context where a lexeme is taken literally. The
* result is passed as a bound parameter regardless.
*/
final class FreeTextCompiler
{
/**
* Below this, a substring needle is both useless and expensive: a trigram
* index cannot serve a pattern shorter than three characters, so "de"
* would degrade to a sequential scan over every body in the account, and
* match half of them anyway.
*/
private const int MIN_SUBSTRING_LENGTH = 3;
public function compile(string $freeText): FreeTextQuery
{
$freeText = trim($freeText);
if ('' === $freeText) {
return new FreeTextQuery('', null, null);
}
// Deliberate websearch syntax is honoured exactly as written. Widening
// it would be the opposite of what quoting and negation are FOR.
if (true === $this->usesWebsearchSyntax($freeText)) {
return new FreeTextQuery($freeText, null, null);
}
$terms = preg_split('/\s+/', $freeText) ?: [];
$lexemes = [];
foreach ($terms as $term) {
$safe = $this->sanitiseLexeme($term);
if ('' !== $safe) {
$lexemes[] = sprintf("'%s':*", $safe);
}
}
$prefix = [] === $lexemes ? null : implode(' & ', $lexemes);
// One term only. For "invoice acme" the substring pass would have to
// OR two needles across four columns and would still not answer the
// question the user asked (both words, same message) any better than
// the tsquery already does.
$substring = null;
if (1 === count($terms) && mb_strlen($freeText) >= self::MIN_SUBSTRING_LENGTH) {
$substring = $freeText;
}
return new FreeTextQuery($freeText, $prefix, $substring);
}
/**
* Escape the wildcards ILIKE would otherwise honour, so a search for "50%"
* looks for "50%" and not for "50 followed by anything".
*/
public function likePattern(string $needle): string
{
return '%' . str_replace(['\\', '%', '_'], ['\\\\', '\\%', '\\_'], $needle) . '%';
}
private function usesWebsearchSyntax(string $freeText): bool
{
if (true === str_contains($freeText, '"')) {
return true;
}
// `or` is websearch's alternation keyword, in any case.
if (1 === preg_match('/(^|\s)or(\s|$)/i', $freeText)) {
return true;
}
// A term that starts with `-` is a negation.
return 1 === preg_match('/(^|\s)-\S/', $freeText);
}
/**
* Keep only what can appear inside a quoted lexeme without ending it.
*
* An allowlist rather than a denylist: `&`, `|`, `!`, `(`, `)`, `:`, `*`
* and the quote itself are all tsquery syntax, and so is whatever the next
* Postgres release adds. Letters, digits and the punctuation that holds
* addresses and hostnames together are what a lexeme is made of; the quote
* cannot survive this, which is what makes the surrounding quoting safe.
*/
private function sanitiseLexeme(string $term): string
{
return preg_replace('/[^\p{L}\p{N}._@+\-\/]+/u', '', $term) ?? '';
}
}
src/Service/Search/FreeTextQuery.php
+31
View File
@@ -0,0 +1,31 @@
<?php
declare(strict_types=1);
namespace App\Service\Search;
/**
* The three shapes one piece of free text has to take to be findable.
*
* `websearch` is the string as typed, for websearch_to_tsquery — it is the only
* one of the three that understands quotes, OR and negation, so it stays the
* primary match and the other two are only ever ADDED to it.
*
* `prefix` is a to_tsquery expression with `:*` on every term, so a half-typed
* word finds the whole one. Null when the text uses websearch syntax: ORing a
* prefix match onto `-foo` would hand back exactly the mail the user excluded.
*
* `substring` is a bare needle for ILIKE, for the words full-text cannot see at
* all — the ones inside a token the tokenizer refused to split. Null unless the
* text is a single term of at least three characters; see SearchStrategy in the
* repository for why that line is drawn there and what it costs.
*/
final readonly class FreeTextQuery
{
public function __construct(
public string $websearch,
public ?string $prefix,
public ?string $substring,
) {
}
}
templates/_layout/app.html.twig
+23 -5
View File
@@ -1,5 +1,16 @@
<!DOCTYPE html>
<html class="{{ appearance_class() }}"
{# `lang` is what tells a screen reader which voice to read the page in, and
what a browser offers to translate from. It was absent entirely, so assistive
technology fell back to the OS language and read a German UI in an English
voice.
The request locale, not the user's stored preference: they agree for a
signed-in user, and for the login page the request locale is the only one
there is. Underscores become hyphens because BCP 47 writes en-PI where
Symfony writes en_PI, and an invalid tag is ignored the same as a missing
one. #}
<html lang="{{ app.request.locale|replace({'_': '-'}) }}"
class="{{ appearance_class() }}"
data-theme="{{ appearance_theme() }}"
style="{{ appearance_vars() }}">
<head>
@@ -7,9 +18,16 @@
{# Without this the browser assumes a ~980px desktop canvas and scales the
whole app down. `viewport-fit=cover` lets the gradient run under the
notch; the safe-area padding in app.css keeps content clear of it.
Zoom is pinned so the app behaves like an installed PWA rather than a
page — see the 16px form-control rule in app.css, which is what actually
stops iOS zooming on focus (it ignores user-scalable).
Zoom is NOT pinned. `maximum-scale=1, user-scalable=no` used to be here
to make the app feel installed rather than page-like; it also took away
pinch-zoom, which WCAG 1.4.4 requires and which is the only way some
people read anything at all. The reason it was added — iOS zooming when
a small form control takes focus — is already handled by the 16px
form-control rule in app.css, which is what actually prevents that; iOS
ignores user-scalable anyway, so this only ever disabled zoom on Android
where nothing was wrong.
`interactive-widget=resizes-content`: Android's default (resizes-visual)
shrinks only the visual viewport for the keyboard and leaves the layout
viewport — where fixed elements live — at full height, so the page
@@ -18,7 +36,7 @@
the keyboard and there is nothing to pan. iOS ignores this keyword; the
compose controller's visualViewport tracking stays as its fallback. #}
<meta name="viewport"
content="width=device-width, initial-scale=1, maximum-scale=1, user-scalable=no, viewport-fit=cover, interactive-widget=resizes-content">
content="width=device-width, initial-scale=1, viewport-fit=cover, interactive-widget=resizes-content">
<title>{% block title %}Welcome!{% endblock %}</title>
{% include '_partials/_favicon.html.twig' %}
{# PWA: iOS only offers Web Push to apps added to the Home Screen, and it
templates/bundles/TwigBundle/Exception/error.html.twig
+69
View File
@@ -0,0 +1,69 @@
{#
The fallback error page, and the body every specific one extends.
Symfony picks errorNNN.html.twig if it exists and falls back to this, so a
status without its own template still lands somewhere branded rather than on
the stock white "Oops! An Error Occurred".
Standalone rather than `extends '_layout/app.html.twig'`, which was the
first attempt and was wrong: the app shell emits `csrf_token('ajax')`, which
reaches for the session, and there is no session on an anonymous request.
An error template that throws while rendering leaves the visitor on Symfony's
bare fallback — so a 404 for a logged-out visitor would have been exactly the
case where the branded page did not appear. An error page must not need
anything that might itself be what broke: no session, no user, no database,
no Mercure, no service worker.
What it keeps is the appearance: the theme functions read a cookie-backed
default and work without a user, so the page is still the user's colours
rather than a white sheet.
#}
{# Even `app.request` is guarded. An error template can be rendered with no
request in scope at all, and a null-property access here would throw inside
the handler for the thing that already threw. #}
{% set error_locale = app.request is not null ? app.request.locale : 'en' %}
<!DOCTYPE html>
<html lang="{{ error_locale|replace({'_': '-'}) }}"
class="{{ appearance_class() }}"
data-theme="{{ appearance_theme() }}"
style="{{ appearance_vars() }}">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1, viewport-fit=cover">
<title>{{ error_title|default('errors.page.server.title'|trans) }} — {{ 'app.name'|trans }}</title>
{% include '_partials/_favicon.html.twig' %}
{{ importmap('app') }}
</head>
<body class="app-bg text-ink">
<main class="min-h-screen flex items-center justify-center px-4">
<div class="w-full max-w-md text-center">
<div class="flex items-center justify-center gap-2 mb-8">
<i class="fa-solid fa-envelope text-accent text-2xl" aria-hidden="true"></i>
<span class="text-xl font-semibold text-ink">{{ 'app.name'|trans }}</span>
</div>
{# The status number is a numeral in every locale this ships in, so
it is not translated — but it is decorative beside the heading
that actually says what happened, and a screen reader is better
served by that heading than by hearing "four hundred and four". #}
<p class="text-6xl font-semibold text-ink-faint tabular-nums mb-4" aria-hidden="true">
{{- error_code|default(status_code|default(500)) -}}
</p>
<h1 class="text-lg font-semibold text-ink mb-2">
{{- error_heading|default('errors.page.server.heading'|trans) -}}
</h1>
<p class="text-sm text-ink-muted mb-8">
{{- error_body|default('errors.page.server.body'|trans) -}}
</p>
<a href="{{ path('app_mail_inbox') }}"
class="inline-flex items-center gap-2 px-4 py-2 rounded-lg bg-accent text-accent-ink text-sm font-medium hover:bg-accent-strong transition-colors">
<i class="fa-solid fa-arrow-left" aria-hidden="true"></i>
{{ 'errors.page.back'|trans }}
</a>
</div>
</main>
</body>
</html>
templates/bundles/TwigBundle/Exception/error404.html.twig
+6
View File
@@ -0,0 +1,6 @@
{% extends 'bundles/TwigBundle/Exception/error.html.twig' %}
{% set error_code = 404 %}
{% set error_title = 'errors.page.not_found.title'|trans %}
{% set error_heading = 'errors.page.not_found.heading'|trans %}
{% set error_body = 'errors.page.not_found.body'|trans %}
templates/bundles/TwigBundle/Exception/error500.html.twig
+13
View File
@@ -0,0 +1,13 @@
{#
The 500 page links to the inbox like the 404 does, which is worth a word:
if what broke is the inbox, that link goes straight back into the fault. It
stays because the alternative — a dead end — leaves someone whose session is
fine after a one-off failure with nowhere to click, and a second attempt is
the thing that usually works.
#}
{% extends 'bundles/TwigBundle/Exception/error.html.twig' %}
{% set error_code = 500 %}
{% set error_title = 'errors.page.server.title'|trans %}
{% set error_heading = 'errors.page.server.heading'|trans %}
{% set error_body = 'errors.page.server.body'|trans %}
templates/mail/original.html.twig
+1 -1
View File
@@ -4,7 +4,7 @@ mail/original.html.twig
Standalone "show original" page (opened in a new tab). Deliberately plain and
independent of the app layout; the source is printed escaped, never |raw.
#}<!DOCTYPE html>
<html lang="en">
<html lang="{{ app.request.locale|replace({'_': '-'}) }}">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
templates/mail/print.html.twig
+1 -1
View File
@@ -4,7 +4,7 @@ mail/print.html.twig
Standalone printable view of a single message (opened in a new tab, fires the
print dialog on load). Only the sanitized body is rendered as markup.
#}<!DOCTYPE html>
<html lang="en">
<html lang="{{ app.request.locale|replace({'_': '-'}) }}">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
templates/search/search.html.twig
+6 -1
View File
@@ -1,7 +1,12 @@
{% extends '_layout/_mailbox.html.twig' %}
{#
The query goes through a translated pattern rather than being concatenated
ahead of the word "Search": the order of the two is a property of the
language, not of the page.
#}
{% block title %}
{% if q %}{{ q }} — {{ 'search.title'|trans }}{% else %}{{ 'search.title'|trans }}{% endif %} — {{ 'app.name'|trans }}
{%- if q %}{{ 'search.title_query'|trans({ '%query%': q }) }}{% else %}{{ 'search.title'|trans }}{% endif %} — {{ 'app.name'|trans -}}
{% endblock %}
{% block inbox_tabs %}
templates/settings/_appearance.html.twig
+7 -1
View File
@@ -28,7 +28,13 @@
<span class="w-2 h-2 rounded-full" style="background:{{ swatch[1] }}"></span>
<span class="w-2 h-2 rounded-full" style="background:{{ swatch[2] }}"></span>
</span>
<span class="text-[11px] text-ink-muted">{{ ('settings.appearance.theme.' ~ theme.value)|trans }}</span>
{# The theme's name is the only thing that says which tile
is which, so it is primary content, not a caption: it
gets the soft ink (7.5:1 or better in every theme) at
12px rather than muted ink at 11px. It was the lowest
contrast text on the page and it is the text a person
reads most carefully here. #}
<span class="text-xs text-ink-soft">{{ ('settings.appearance.theme.' ~ theme.value)|trans }}</span>
</button>
{% endfor %}
</div>
templates/sharing/_layout.html.twig
+1 -1
View File
@@ -44,7 +44,7 @@ Variables:
appearance PublicAppearance the owner's theme, or Paper by default
#}
<!DOCTYPE html>
<html lang="{{ app.request.locale }}"
<html lang="{{ app.request.locale|replace({'_': '-'}) }}"
class="{{ appearance is defined ? appearance.htmlClass : '' }}"
data-theme="{{ appearance is defined ? appearance.theme : 'paper' }}"
style="{{ appearance is defined ? appearance.cssVariables : '' }}">
tests/Controller/ErrorAndPaginationTest.php
+188
View File
@@ -0,0 +1,188 @@
<?php
declare(strict_types=1);
namespace App\Tests\Controller;
use App\Repository\User\UserRepository;
use PHPUnit\Framework\Attributes\DataProvider;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
/**
* The URLs a person can produce by editing the address bar, and what they
* should answer.
*
* All of these were 500s or nonsense pages. They share a cause worth naming:
* every one of them is a value that arrived as text in a URL and was handed
* straight to something that assumed it was already valid — an enum, an entity
* id, an offset. A URL is user input with no form around it and no validator
* attached, which is exactly why it is the input that gets forgotten.
*/
final class ErrorAndPaginationTest extends WebTestCase
{
private const string ADMIN_EMAIL = 'e2e-admin@plmail.test';
/**
* A non-numeric id must not reach the entity resolver.
*
* @return iterable<string, array{string}>
*/
public static function nonNumericIds(): iterable
{
yield 'thread' => ['/mail/thread/abc'];
yield 'message' => ['/mail/message/abc'];
yield 'label' => ['/mail/label/abc'];
yield 'account' => ['/mail/account/abc'];
yield 'attachment' => ['/mail/attachment/abc'];
}
#[DataProvider('nonNumericIds')]
public function testNonNumericIdIsNotFoundRatherThanServerError(string $path): void
{
$client = $this->loggedInClient();
$client->request('GET', $path);
self::assertSame(404, $client->getResponse()->getStatusCode(), $path);
}
/**
* An unknown inbox tab is a mistyped or stale link, not a fault. It used to
* be MessageCategory::from() and therefore a ValueError and therefore a 500.
*
* @return iterable<string, array{string}>
*/
public static function unknownTabs(): iterable
{
yield 'nonsense' => ['/mail/inbox?tab=quatsch'];
yield 'empty' => ['/mail/inbox?tab='];
yield 'numeric' => ['/mail/inbox?tab=7'];
}
#[DataProvider('unknownTabs')]
public function testUnknownInboxTabFallsBackToPrimary(string $path): void
{
$client = $this->loggedInClient();
$client->request('GET', $path);
self::assertSame(200, $client->getResponse()->getStatusCode(), $path);
}
/**
* Past the last page, the list redirects back onto a page that exists.
*
* The count is whatever the seeded database holds, which is why this asserts
* on the redirect rather than on a row count: the invariant is that the URL
* stops claiming a page number the list cannot serve, at any list size.
*
* @return iterable<string, array{string, string}>
*/
public static function pagesPastTheEnd(): iterable
{
yield 'inbox' => ['/mail/inbox?page=999', '/mail/inbox'];
yield 'trash' => ['/mail/trash?page=999', '/mail/trash'];
yield 'sent' => ['/mail/sent?page=5', '/mail/sent'];
yield 'drafts' => ['/mail/drafts?page=5', '/mail/drafts'];
}
#[DataProvider('pagesPastTheEnd')]
public function testPagePastTheEndRedirectsOntoTheList(string $path, string $expectedPrefix): void
{
$client = $this->loggedInClient();
$client->request('GET', $path);
$response = $client->getResponse();
self::assertTrue($response->isRedirect(), $path . ' should redirect');
self::assertStringStartsWith(
$expectedPrefix,
(string) $response->headers->get('Location'),
$path,
);
// And the page it lands on renders.
$client->followRedirect();
self::assertSame(200, $client->getResponse()->getStatusCode(), $path);
}
/**
* The lower bound was always clamped and must stay clamped — `?page=0` and
* `?page=-3` render page one rather than computing a negative offset.
*/
public function testPageBelowOneStillRendersTheFirstPage(): void
{
$client = $this->loggedInClient();
foreach (['/mail/inbox?page=0', '/mail/inbox?page=-3', '/mail/inbox?page=nonsense'] as $path) {
$client->request('GET', $path);
self::assertSame(200, $client->getResponse()->getStatusCode(), $path);
}
}
/**
* A page that already exists is left alone. Without this the redirect could
* be unconditional and every test above would still pass.
*/
public function testAnExistingPageIsNotRedirected(): void
{
$client = $this->loggedInClient();
$client->request('GET', '/mail/inbox');
self::assertSame(200, $client->getResponse()->getStatusCode());
}
/**
* The error templates render, and render translated.
*
* Rendered directly rather than by provoking a real error: with debug on —
* which is how the test kernel runs — Symfony shows the exception page and
* these templates never execute, so the only way to catch a typo in one is
* to ask Twig for it.
*
* @return iterable<string, array{string, string}>
*/
public static function errorTemplates(): iterable
{
yield '404' => ['bundles/TwigBundle/Exception/error404.html.twig', 'errors.page.not_found.heading'];
yield '500' => ['bundles/TwigBundle/Exception/error500.html.twig', 'errors.page.server.heading'];
yield 'fallback' => ['bundles/TwigBundle/Exception/error.html.twig', 'errors.page.server.heading'];
}
#[DataProvider('errorTemplates')]
public function testErrorTemplateRendersBrandedAndTranslated(string $template, string $key): void
{
self::bootKernel();
$container = static::getContainer();
$translator = $container->get('translator');
$html = $container->get('twig')->render($template);
$heading = $translator->trans($key);
self::assertStringContainsString($heading, $html, 'the localised heading is missing');
self::assertNotSame($key, $heading, 'the translation key has no entry');
// Branded, not the stock Symfony page: it carries the app shell and a
// way back, and no raw translation key leaked into the markup.
self::assertStringContainsString('/mail/inbox', $html);
self::assertStringNotContainsString('errors.page.', $html);
}
private function loggedInClient(): \Symfony\Bundle\FrameworkBundle\KernelBrowser
{
$client = static::createClient();
$user = static::getContainer()->get(UserRepository::class)
->findOneBy(['email' => self::ADMIN_EMAIL]);
if (null === $user) {
self::markTestSkipped('run `app:test:seed-user --admin` first');
}
$client->loginUser($user);
return $client;
}
}
tests/Repository/Mail/MessageSearchTest.php
+83
View File
@@ -18,6 +18,7 @@ use App\Service\Search\SearchQueryParser;
use DateTimeImmutable;
use Doctrine\DBAL\Connection;
use Doctrine\ORM\EntityManagerInterface;
use PHPUnit\Framework\Attributes\DataProvider;
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
/**
@@ -121,6 +122,88 @@ final class MessageSearchTest extends KernelTestCase
self::assertSame(['Notes'], $this->search('meetings'));
}
/**
* The table from the bug report, verbatim.
*
* One mail, five ways of reaching for it, four of which used to come back
* empty. Kept as one test over one fixture because that is the shape of the
* complaint: the SAME mail, visible on screen, unfindable by every spelling
* of what is visibly in it except the one nobody would type.
*
* The two failures have different causes and different fixes, which is why
* both a prefix pass and a substring pass exist:
*
* "Testmai" a prefix of a lexeme. `:*` reaches it.
* "wirhub" NOT a prefix of anything — the tokenizer emits
* "help.wirhub.de" as one host lexeme, so it is a substring
* of a lexeme, and only ILIKE reaches it.
*/
#[DataProvider('reportedSearches')]
public function testTheReportedSearchesAllFindTheMail(string $query): void
{
$this->seedMessage(
subject: 'Testmail Betreff',
body: 'Bitte besuche help.wirhub.de fuer mehr Infos',
);
$this->seedMessage(subject: 'Unrelated', body: 'Nothing in particular.');
self::assertSame(['Testmail Betreff'], $this->search($query));
}
/** @return iterable<string, array{string}> */
public static function reportedSearches(): iterable
{
yield 'whole word, which already worked' => ['Testmail'];
yield 'prefix of a subject word' => ['Testmai'];
yield 'inside a host token' => ['wirhub'];
yield 'a suffix of a host token' => ['wirhub.de'];
yield 'the host token entire' => ['help.wirhub.de'];
}
/**
* Widening free text must not widen a query that was narrowed on purpose.
*
* Negation is the one that matters: OR-ing a prefix match onto `-invoice`
* hands back precisely the mail the user asked not to see, which is worse
* than the bug being fixed here. So websearch syntax turns the extra passes
* off rather than fighting them.
*/
public function testDeliberateWebsearchSyntaxIsNotWidened(): void
{
$this->seedMessage(subject: 'Invoice reminder', body: 'Payment for the invoice.');
$this->seedMessage(subject: 'Payment plan', body: 'A plan for payment.');
self::assertSame(['Payment plan'], $this->search('payment -invoice'));
self::assertSame(['Payment plan'], $this->search('"payment plan"'));
self::assertCount(2, $this->search('invoice OR plan'));
}
/**
* tsquery syntax typed into the box is text, not syntax. Before the prefix
* pass there was nothing to escape — websearch_to_tsquery is total over its
* input — so this is new surface and worth pinning: any of these reaching
* to_tsquery unescaped is a 500, and `!` reaching it is a search that
* returns the complement of what was asked for.
*/
#[DataProvider('hostileQueries')]
public function testTsqueryPunctuationIsTextNotSyntax(string $query): void
{
$this->seedMessage(subject: 'Invoice reminder', body: 'Payment for the invoice.');
self::assertIsArray($this->search($query));
}
/** @return iterable<string, array{string}> */
public static function hostileQueries(): iterable
{
yield 'bare colon star' => ['invoice:*'];
yield 'unbalanced quote' => ["invoice'"];
yield 'boolean operators' => ['invoice & payment'];
yield 'negation operator' => ['!invoice'];
yield 'unbalanced paren' => ['(invoice'];
yield 'nothing but syntax' => ['&|!():*'];
}
public function testLabelMatchesAUserLabelByName(): void
{
$receipts = $this->seedLabel('Receipts');
tests/Service/Appearance/AccentInkContrastTest.php
+188
View File
@@ -0,0 +1,188 @@
<?php
declare(strict_types=1);
namespace App\Tests\Service\Appearance;
use App\Domain\Enum\Theme\Theme;
use App\Entity\Embeddable\Appearance;
use App\Service\Appearance\AppearanceRenderer;
use App\Service\Appearance\BackgroundResolver;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Asset\Packages;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
/**
* Whatever accent a user picks, the text on it is readable.
*
* The accent is chosen from a colour wheel, so this cannot be checked by
* inspecting a palette — the palette is whatever someone liked. It has to be a
* property of the rule, which is why the sweep below exists: it walks the
* accent space rather than a list of colours somebody thought of.
*
* The reported failure was white on a pink accent at 2.34:1. The cause was an
* ink chosen by a threshold over raw, un-expanded channel values, which is not
* luminance and crossed over in the wrong place.
*/
final class AccentInkContrastTest extends TestCase
{
private const float AA_BODY_TEXT = 4.5;
/**
* Every accent in a coarse sweep of the RGB cube gets a readable ink.
*
* 6^3 = 216 colours at even spacing. Not exhaustive, but the failure mode
* being guarded is a whole REGION of the space getting the wrong ink — the
* old rule was wrong for most of the mid-tones — and a region that size
* cannot hide between these samples.
*/
public function testEveryAccentInTheCubeGetsReadableInk(): void
{
$worst = 21.0;
$worstHex = '';
$steps = [0, 51, 102, 153, 204, 255];
foreach ($steps as $r) {
foreach ($steps as $g) {
foreach ($steps as $b) {
$hex = sprintf('#%02X%02X%02X', $r, $g, $b);
$ratio = $this->contrastForAccent($hex);
if ($ratio < $worst) {
$worst = $ratio;
$worstHex = $hex;
}
}
}
}
self::assertGreaterThanOrEqual(
self::AA_BODY_TEXT,
$worst,
sprintf('worst accent was %s at %.2f:1', $worstHex, $worst),
);
}
/**
* The colours from the report and its neighbourhood, named so a regression
* says which one broke.
*
* @return iterable<string, array{string}>
*/
public static function troublesomeAccents(): iterable
{
yield 'the reported pink' => ['#F472B6'];
yield 'a stronger pink' => ['#EC4899'];
yield 'mid grey' => ['#767676'];
// Straddles the old 0.6 threshold, where the previous rule flipped.
yield 'mid teal' => ['#3AA6A6'];
yield 'amber' => ['#F59E0B'];
yield 'lime' => ['#84CC16'];
yield 'deep indigo' => ['#312E81'];
}
#[DataProvider('troublesomeAccents')]
public function testNamedAccentsMeetAA(string $hex): void
{
self::assertGreaterThanOrEqual(
self::AA_BODY_TEXT,
$this->contrastForAccent($hex),
$hex,
);
}
/**
* A light accent must get dark ink and a dark accent light ink. Contrast
* alone would be satisfied by always picking the further-away ink, so this
* pins the direction too — and it is the direction that was wrong.
*/
public function testLightAccentsGetDarkInkAndDarkAccentsGetLightInk(): void
{
self::assertSame([24, 24, 27], $this->inkFor('#FDE68A'), 'pale amber needs dark ink');
self::assertSame([24, 24, 27], $this->inkFor('#F472B6'), 'the reported pink needs dark ink');
self::assertSame([255, 255, 255], $this->inkFor('#312E81'), 'deep indigo needs light ink');
self::assertSame([255, 255, 255], $this->inkFor('#000000'), 'black needs light ink');
}
/**
* Every stock theme's own accent is readable too — the themes seed the
* user's accent setting from their swatch, so a theme ships whatever this
* rule then has to cope with.
*/
public function testEveryStockThemeAccentIsReadable(): void
{
foreach (Theme::cases() as $theme) {
$accent = $theme->defaults()['accent'];
self::assertGreaterThanOrEqual(
self::AA_BODY_TEXT,
$this->contrastForAccent($accent),
$theme->value . ' (' . $accent . ')',
);
}
}
/** @return array{0:int,1:int,2:int} */
private function inkFor(string $hex): array
{
$appearance = new Appearance();
$appearance->accent = $hex;
// The background resolver is irrelevant here — the default appearance
// uses the theme background, for which it returns null without touching
// either collaborator — but the renderer's constructor still wants one.
$renderer = new AppearanceRenderer(new BackgroundResolver(
$this->createStub(Packages::class),
$this->createStub(UrlGeneratorInterface::class),
));
$vars = $renderer->cssVariables($appearance);
preg_match('/--rgb-accent-ink:(\d+) (\d+) (\d+)/', $vars, $m);
self::assertNotEmpty($m, 'no --rgb-accent-ink was written for ' . $hex);
return [(int) $m[1], (int) $m[2], (int) $m[3]];
}
private function contrastForAccent(string $hex): float
{
return $this->ratio($this->luminance($this->channels($hex)), $this->luminance($this->inkFor($hex)));
}
/** @return array{0:int,1:int,2:int} */
private function channels(string $hex): array
{
$hex = ltrim($hex, '#');
return [
(int) hexdec(substr($hex, 0, 2)),
(int) hexdec(substr($hex, 2, 2)),
(int) hexdec(substr($hex, 4, 2)),
];
}
/**
* Written out here rather than shared with the renderer on purpose: a test
* that reuses the implementation's own maths agrees with it by
* construction, including when both are wrong.
*
* @param array{0:int,1:int,2:int} $rgb
*/
private function luminance(array $rgb): float
{
$expand = static function (int $c): float {
$s = $c / 255;
return $s <= 0.04045 ? $s / 12.92 : (($s + 0.055) / 1.055) ** 2.4;
};
return 0.2126 * $expand($rgb[0]) + 0.7152 * $expand($rgb[1]) + 0.0722 * $expand($rgb[2]);
}
private function ratio(float $a, float $b): float
{
return (max($a, $b) + 0.05) / (min($a, $b) + 0.05);
}
}
tests/Service/Appearance/ThemeInkContrastTest.php
+193
View File
@@ -0,0 +1,193 @@
<?php
declare(strict_types=1);
namespace App\Tests\Service\Appearance;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase;
/**
* Every tier of every theme's ink ramp is readable on its own surface, and the
* tiers stay in order.
*
* Two separate promises, both of which had been broken:
*
* Readable. --rgb-ink-faint was 2.56:1 on white — it is the colour the 11px
* labels are drawn in, and it was little more than half of what WCAG AA asks
* for body text. Every theme failed this at the faint tier and several at the
* muted one.
*
* In order. Fixing the first can break the second, and did: darkening solar's
* faint and muted tiers pushed them PAST its own ink and ink-soft, so the
* ramp ran backwards and "less important" text came out stronger than the
* text it was subordinate to. Contrast alone is not the property wanted; a
* legible ordered ramp is.
*
* Read out of the stylesheet rather than from a table kept beside it, for the
* reason ThemeVariableCompletenessTest gives: a table beside it goes stale.
*/
final class ThemeInkContrastTest extends TestCase
{
/** WCAG 2.x AA for body text, which is what these tiers draw. */
private const float AA_BODY_TEXT = 4.5;
/**
* Each ramp: the four ink tiers and the surface they sit on.
*
* @var array<string, array{list<string>, string}>
*/
private const array RAMPS = [
'page' => [
['--rgb-ink', '--rgb-ink-soft', '--rgb-ink-muted', '--rgb-ink-faint'],
'--rgb-surface',
],
'reading sheet' => [
['--rgb-sheet-ink', '--rgb-sheet-ink-soft', '--rgb-sheet-ink-muted', '--rgb-sheet-ink-faint'],
'--rgb-sheet',
],
];
/** @return iterable<string, array{string, string}> */
public static function ramps(): iterable
{
foreach (array_keys(self::palettes()) as $selector) {
foreach (array_keys(self::RAMPS) as $ramp) {
yield $selector . ' / ' . $ramp => [$selector, $ramp];
}
}
}
#[DataProvider('ramps')]
public function testEveryInkTierMeetsAAOnItsSurface(string $selector, string $ramp): void
{
[$tiers, $surfaceVar] = self::RAMPS[$ramp];
$palette = self::palettes()[$selector];
$surface = $palette[$surfaceVar];
foreach ($tiers as $tier) {
$ratio = self::ratio($palette[$tier], $surface);
self::assertGreaterThanOrEqual(
self::AA_BODY_TEXT,
$ratio,
sprintf('%s %s on %s is %.2f:1', $selector, $tier, $surfaceVar, $ratio),
);
}
}
#[DataProvider('ramps')]
public function testTheRampRunsFromStrongestToFaintest(string $selector, string $ramp): void
{
[$tiers, $surfaceVar] = self::RAMPS[$ramp];
$palette = self::palettes()[$selector];
$surface = $palette[$surfaceVar];
$ratios = array_map(
static fn (string $tier): float => self::ratio($palette[$tier], $surface),
$tiers,
);
for ($i = 1; $i < count($ratios); $i++) {
self::assertLessThan(
$ratios[$i - 1],
$ratios[$i],
sprintf(
'%s %s: %s (%.2f:1) is not fainter than %s (%.2f:1)',
$selector,
$ramp,
$tiers[$i],
$ratios[$i],
$tiers[$i - 1],
$ratios[$i - 1],
),
);
}
}
/**
* The channel triples of every palette block that declares a surface.
*
* @return array<string, array<string, array{0:int,1:int,2:int}>>
*/
private static function palettes(): array
{
static $cache = null;
if (null !== $cache) {
return $cache;
}
$css = (string) preg_replace('#/\*.*?\*/#s', '', self::stylesheet());
preg_match_all(
'/(:root|\.dark|\[data-theme="[a-z]+"\])\s*\{([^}]*)\}/',
$css,
$matches,
PREG_SET_ORDER,
);
$palettes = [];
foreach ($matches as $match) {
preg_match_all(
'/(--rgb-[a-z-]+):\s*(\d+)\s+(\d+)\s+(\d+)\s*;/',
$match[2],
$vars,
PREG_SET_ORDER,
);
$palette = [];
foreach ($vars as $var) {
$palette[$var[1]] = [(int) $var[2], (int) $var[3], (int) $var[4]];
}
// Blocks that carry no surface are not palettes — utilities and
// the like. Only a block that paints a background can be measured
// against one.
if (isset($palette['--rgb-surface'])) {
$palettes[$match[1]] = $palette;
}
}
self::assertNotEmpty($palettes, 'no palette blocks found in app.css');
return $cache = $palettes;
}
private static function stylesheet(): string
{
$path = dirname(__DIR__, 3) . '/assets/styles/app.css';
self::assertFileExists($path);
return (string) file_get_contents($path);
}
/**
* @param array{0:int,1:int,2:int} $a
* @param array{0:int,1:int,2:int} $b
*/
private static function ratio(array $a, array $b): float
{
$la = self::luminance($a);
$lb = self::luminance($b);
return (max($la, $lb) + 0.05) / (min($la, $lb) + 0.05);
}
/** @param array{0:int,1:int,2:int} $rgb */
private static function luminance(array $rgb): float
{
$expand = static function (int $channel): float {
$c = $channel / 255;
return $c <= 0.04045 ? $c / 12.92 : (($c + 0.055) / 1.055) ** 2.4;
};
return 0.2126 * $expand($rgb[0]) + 0.7152 * $expand($rgb[1]) + 0.0722 * $expand($rgb[2]);
}
}
translations/messages.de.yaml
+12
View File
@@ -1091,6 +1091,16 @@ drafts:
errors:
access_denied: Du hast keinen Zugriff auf diese Ressource.
not_found: Die angeforderte Ressource wurde nicht gefunden.
page:
back: Zurück zu deiner Mail
not_found:
body: Die aufgerufene Seite gibt es hier nicht. Vielleicht ist der Link vertippt, vielleicht wurde das Ziel verschoben oder gelöscht.
heading: Unter dieser Adresse ist nichts
title: Seite nicht gefunden
server:
body: Bei uns ist etwas schiefgelaufen, nicht bei dir. Der Fehler wurde protokolliert; ein neuer Versuch gleich hilft meistens.
heading: Da ist etwas schiefgelaufen
title: Serverfehler
form:
file:
@@ -1422,6 +1432,8 @@ search:
label: Sortieren nach
recent: Neueste zuerst
relevance: Relevanteste zuerst
title: Suche
title_query: 'Suche: %query%'
sent:
empty: Keine gesendeten Nachrichten.
translations/messages.en.yaml
+12
View File
@@ -1064,6 +1064,16 @@ drafts:
errors:
access_denied: You do not have access to this resource.
not_found: The requested resource could not be found.
page:
back: Back to your mail
not_found:
body: The page you asked for is not here. The link may be mistyped, or what it pointed at may have been moved or deleted.
heading: Nothing at this address
title: Page not found
server:
body: Something went wrong at our end, not yours. The fault has been logged; trying again in a moment usually works.
heading: Something went wrong
title: Server error
form:
file:
@@ -1392,6 +1402,8 @@ search:
label: Sort by
recent: Most recent
relevance: Most relevant
title: Search
title_query: 'Search: %query%'
sent:
empty: No sent messages.
translations/messages.en_PI.yaml
+12
View File
@@ -809,6 +809,16 @@ drafts:
errors:
access_denied: Ye have no leave to enter here.
not_found: That be nowhere on the charts.
page:
back: Back to yer mail
not_found:
body: The page ye hailed be not here. Happen the link be mistyped, or what it pointed at were moved or scuttled.
heading: Naught at these coordinates
title: Page be lost at sea
server:
body: Somethin' went wrong on our deck, not yours. The fault be in the log; another go in a moment most often sails.
heading: Somethin' went wrong
title: The ship be takin' water
form:
file:
@@ -1128,6 +1138,8 @@ search:
label: Sort by
recent: Freshest catch
relevance: Closest to the mark
title: Scourin'
title_query: 'Scourin'' for: %query%'
sent:
empty: No bottles cast off.
worktree-agent-aeee376464efa3888/get.sh
+5
View File
@@ -0,0 +1,5 @@
#!/bin/sh
# Authenticated GET against the live plm_d stack. $1 = path, rest passed to curl.
J=/tmp/claude-1000/-home-karatektus-plmailstuff/b28e1914-8509-423e-8a57-1bb9498e8e5c/scratchpad/c2.txt
P="$1"; shift
exec curl -s -b "$J" -c "$J" -H 'Sec-Fetch-Site: same-origin' "$@" "http://127.0.0.1:8013$P"
worktree-agent-aeee376464efa3888/probe.sh
+6
View File
@@ -0,0 +1,6 @@
#!/bin/sh
D=/home/karatektus/plmailstuff/pl_mail/.claude/worktrees/agent-aeee376464efa3888/worktree-agent-aeee376464efa3888
for p in "$@"; do
printf '%s -> ' "$p"
"$D/get.sh" "$p" -o /dev/null -w '%{http_code}\n'
done
worktree-agent-aeee376464efa3888/run.sh
+12
View File
@@ -0,0 +1,12 @@
#!/bin/sh
# One-off container on the running test stack's network, worktree bind-mounted.
cd /home/karatektus/plmailstuff/pl_mail/.claude/worktrees/agent-aeee376464efa3888
exec docker run --rm --network pl_mail_test_default \
-v "$PWD":/app -w /app \
-e APP_ENV=test -e APP_DEBUG=1 \
-e APP_SECRET=282065e0f71bf96c0dbcf0496c1f5d3e \
-e APP_ENCRYPTION_KEY=emL6R8zs0pSXCVV2HvVvbFubriWJIla7kgnqSGqCJ8E= \
-e "DATABASE_URL=postgresql://app:app@database:5432/app?serverVersion=18&charset=utf8" \
-e MESSENGER_TRANSPORT_DSN=in-memory:// \
-e MAILER_DSN=null://null \
--entrypoint docker-php-entrypoint pl_mail_test-app "$@"
worktree-agent-aeee376464efa3888/search_probe.sh
+7
View File
@@ -0,0 +1,7 @@
#!/bin/sh
# Count result rows the search page returns for each query given.
D=/home/karatektus/plmailstuff/pl_mail/.claude/worktrees/agent-aeee376464efa3888/worktree-agent-aeee376464efa3888
for q in "$@"; do
n=$("$D/get.sh" "/mail/search?q=$q" | grep -oE '/mail/thread/[0-9]+' | sort -u | wc -l)
printf '%-24s %s\n' "$q" "$n"
done