fix(mail): long addresses stay inside the details panel, and replies reach GitHub

A GitHub notification's details panel ran its Reply-To out past the
panel's edge. The value column was a bare 1fr, which never goes narrower
than its longest unbreakable word, so the reply address widened the
whole column, every row with it, to 831px inside a 446px panel, and
break-words never got to wrap anything.

  _message_details    minmax(0,1fr), and min-w-0 on the address text,
                      which is a flex item with the same floor; the
                      comma between two addresses rides with the first,
                      so it no longer wraps onto a line of its own
  message_details_    the panel moves left, and narrows only if it must,
  controller          to stay inside the reading pane: beside the docked
                      calendar it ran 60px past it and was clipped
  AddressHelper       isValidEmail() holds a local part past 64
                      characters to the dot-atom instead of refusing it

That last one is a bug found on the same message. filter_var() enforces
RFC 5321's 64-character local part, and GitHub's reply addresses run to
about ninety. Refused, the reply fell back to the From address,
notifications@github.com, which accepts nothing; with the parser fixed
alone, the address still could not become the contact the To field
needs, and the reply opened with nobody to send to. Every caller asks
the same question, so the one rule changed. Reply-To in the panel is
now parsed like the rows around it, with a copy button.

The new E2E case fails on the old front end (60px past the pane, 382px
past the panel) and passes now.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Paul LütznerandClaude Opus 5.5 committed 2026-09-25 00:13:55 +02:00
1 parent 5effde6498
commit 13a175a118
9 files changed
+234 -15

No files matched your search

@@ -27,6 +27,7 @@ export default class extends Controller {
}
this.panelTarget.classList.remove("hidden");
this.#fitIntoPane();
this.#header()?.setAttribute("data-details-open", "");
if (this.hasCaretTarget) {
@@ -66,6 +67,57 @@ export default class extends Controller {
// ── Private ───────────────────────────────────────────────────────────
/**
* Keep the panel inside the pane it opens in.
*
* It hangs from the recipient line at a fixed 28rem, and the reading pane
* clips whatever leaves it. Beside the docked calendar the pane is
* narrower than the line's offset plus the panel, so the right edge of
* every row was cut away, addresses included. So the panel moves left by
* what does not fit, and is narrowed only when the pane itself is narrower
* than the panel. Measured on every open, since the calendar can be docked
* or closed in between.
*/
#fitIntoPane() {
const panel = this.panelTarget;
const pane = this.#clippingAncestor();
panel.style.left = "";
panel.style.maxWidth = "";
if (null === pane) {
return;
}
const margin = 8;
const bounds = pane.getBoundingClientRect();
const room = bounds.width - 2 * margin;
if (panel.getBoundingClientRect().width > room) {
panel.style.maxWidth = `${room}px`;
}
const box = panel.getBoundingClientRect();
const overflow = box.right - (bounds.right - margin);
if (overflow > 0) {
panel.style.left = `${-Math.min(overflow, box.left - (bounds.left + margin))}px`;
}
}
/** The nearest ancestor that clips: the one the panel has to fit inside. */
#clippingAncestor() {
for (let element = this.element.parentElement; null !== element; element = element.parentElement) {
const style = getComputedStyle(element);
if ("visible" !== style.overflowX || "visible" !== style.overflowY) {
return element;
}
}
return null;
}
/**
* The sticky header this panel hangs from, where there is one — the
* single-message view includes the same partial outside a conversation and
+17 -2
View File
@@ -58,6 +58,7 @@ final class SeedHeaderCasesCommand extends Command
private const string UNDISCLOSED_SUBJECT = 'E2E Undisclosed';
private const string HEADER_ONLY_SUBJECT = 'E2E Header Only';
private const string LONG_REPLY_SUBJECT = 'E2E Long Reply-To';
public function __construct(
private readonly EntityManagerInterface $entityManager,
@@ -99,7 +100,7 @@ final class SeedHeaderCasesCommand extends Command
$account = $this->account($user);
foreach ([self::UNDISCLOSED_SUBJECT, self::HEADER_ONLY_SUBJECT] as $subject) {
foreach ([self::UNDISCLOSED_SUBJECT, self::HEADER_ONLY_SUBJECT, self::LONG_REPLY_SUBJECT] as $subject) {
$this->removePreviousSeed($account, $subject);
}
@@ -125,9 +126,23 @@ final class SeedHeaderCasesCommand extends Command
],
);
// A GitHub notification's Reply-To: one unbreakable word of about a
// hundred characters, with a local part past RFC 5321's 64. It widened
// the details panel's value column past the panel itself, and the
// parser refused it outright.
$this->seed(
$account,
self::LONG_REPLY_SUBJECT,
'Seeded message whose Reply-To is a GitHub reply address.',
[
'to' => 'karatektus/sigeko <sigeko@noreply.github.com>',
'reply-to' => 'karatektus/sigeko <reply+017babb4f3213243ecf35c1492ef42f159c1db898253598292cf000000011822707092a169ce172b972e@reply.github.com>',
],
);
$this->entityManager->flush();
$io->success('Seeded 2 threads: undisclosed recipients, and header-only recipients.');
$io->success('Seeded 3 threads: undisclosed recipients, header-only recipients, and a long Reply-To.');
return Command::SUCCESS;
}
+27 -3
View File
@@ -21,6 +21,13 @@ final class AddressHelper
/** RFC 5321 §4.5.3.1: 64-octet local part + "@" + 255-octet domain. */
public const int MAX_EMAIL_LENGTH = 320;
/**
* RFC 5322's dot-atom: atoms of the characters an unquoted local part may
* use, joined by single dots. What isValidEmail() holds a local part past
* 64 characters to. See there.
*/
private const string DOT_ATOM = '/^[a-z0-9!#$%&\'*+\/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&\'*+\/=?^_`{|}~-]+)*$/i';
/**
* Display name as a human would write it: encoded words decoded, the
* RFC 5322 quoted-string wrapper removed and its escapes undone.
@@ -77,14 +84,31 @@ final class AddressHelper
* `filter_var` is deliberately stricter than the sync paths are: a header
* that failed to parse yields fragments like `"Doe` or an empty local part,
* and those must not become contacts.
*
* Except in one rule, which real senders break: filter_var() also holds
* the local part to RFC 5321's 64 characters, and GitHub's reply addresses
* run to about ninety. Refused, a reply to a GitHub notification could
* neither go to its Reply-To nor become the contact the To field needs, so
* it opened with nobody to send to. A longer local part is held to the
* dot-atom instead, the unquoted form every such address uses, and its
* domain to filter_var() as before. Checking a shortened copy would not
* do: whatever was cut off would go unchecked.
*/
public static function isValidEmail(?string $raw): bool
{
$email = self::email($raw);
$at = strrpos($email, '@');
return '' !== $email
&& strlen($email) <= self::MAX_EMAIL_LENGTH
&& false !== filter_var($email, FILTER_VALIDATE_EMAIL);
if ('' === $email || strlen($email) > self::MAX_EMAIL_LENGTH || false === $at) {
return false;
}
if ($at <= 64) {
return false !== filter_var($email, FILTER_VALIDATE_EMAIL);
}
return 1 === preg_match(self::DOT_ATOM, substr($email, 0, $at))
&& false !== filter_var('x' . substr($email, $at), FILTER_VALIDATE_EMAIL);
}
/**
+6 -4
View File
@@ -40,16 +40,18 @@ final class MessageRecipientsExtension extends AbstractExtension
}
/**
* @param 'to'|'cc'|'bcc' $field
* @param 'to'|'cc'|'bcc'|'reply-to' $field
*
* @return list<array{name: string, address: string}>
*/
public function addresses(Message $message, string $field): array
{
$stored = match ($field) {
'cc' => $message->ccAddresses,
'bcc' => $message->bccAddresses,
default => $message->toAddresses,
'cc' => $message->ccAddresses,
'bcc' => $message->bccAddresses,
// No column at all: Reply-To only ever lives in the header bag.
'reply-to' => [],
default => $message->toAddresses,
} ?? [];
if ([] !== $stored) {
+24 -6
View File
@@ -25,12 +25,14 @@ mailed-by and signed-by were silently missing from every IMAP message.
{# One address, with a copy button for the addr-spec itself. The clipboard
source is the address text as it is rendered, so the insecure-context
fallback (which selects the source) highlights exactly what it copied. #}
{% macro address(entry) %}
{% macro address(entry, separator = '') %}
<span data-controller="ui--clipboard"
data-ui--clipboard-confirm-text-value="{{ 'clipboard.copied'|trans }}"
data-ui--clipboard-failed-text-value="{{ 'clipboard.copy_failed'|trans }}"
class="inline-flex items-baseline gap-1 max-w-full">
<span class="break-words">
{# min-w-0 for the same reason as the grid column below: a flex item
will not go narrower than its longest word unless told to. #}
<span class="min-w-0 break-words">
{%- if entry.name -%}
{{ entry.name }} &lt;<span data-ui--clipboard-target="source">{{ entry.address }}</span>&gt;
{%- else -%}
@@ -49,13 +51,17 @@ mailed-by and signed-by were silently missing from every IMAP message.
and this is the same news for a screen reader. #}
<span data-ui--clipboard-target="label" class="sr-only" role="status"></span>
</button>
{{- separator -}}
</span>
{% endmacro %}
{# The comma goes inside the address it follows and the space outside, so a
line can break between two addresses but never leave the comma alone at
the start of one, which it did on a phone. #}
{% macro addresses(list) %}
{%- import _self as m -%}
{%- for entry in list -%}
{{ m.address(entry) }}{{ not loop.last ? ', ' : '' }}
{{ m.address(entry, not loop.last ? ',' : '') }}{{ not loop.last ? ' ' : '' }}
{%- endfor -%}
{% endmacro %}
@@ -71,6 +77,7 @@ mailed-by and signed-by were silently missing from every IMAP message.
{% set toList = message_addresses(message, 'to') %}
{% set ccList = message_addresses(message, 'cc') %}
{% set bccList = message_addresses(message, 'bcc') %}
{% set replyToList = message_addresses(message, 'reply-to') %}
{% set summary = message_recipient_summary(message) %}
{% set replyTo = h['reply-to']|default(null) %}
{% set returnPath = h['return-path']|default('') %}
@@ -104,7 +111,9 @@ mailed-by and signed-by were silently missing from every IMAP message.
{% set addressText = a => a.name ? a.name ~ ' <' ~ a.address ~ '>' : a.address %}
{% set headerLines = ['From: ' ~ fromText] %}
{% if replyTo %}
{% set headerLines = headerLines|merge(['Reply-To: ' ~ replyTo]) %}
{% set headerLines = headerLines|merge(['Reply-To: ' ~ (replyToList is not empty
? replyToList|map(addressText)|join(', ')
: replyTo)]) %}
{% endif %}
{% set headerLines = headerLines|merge([
'To: ' ~ (toList is not empty
@@ -153,15 +162,24 @@ mailed-by and signed-by were silently missing from every IMAP message.
border border-line shadow-pop
text-xs cursor-default"
>
<dl class="grid grid-cols-[5.5rem_1fr] gap-x-3 gap-y-1.5 px-4 py-3">
{# minmax(0,1fr), not 1fr. A bare 1fr never goes narrower than its
longest unbreakable word, so a GitHub reply address widened the whole
column past the panel, every row with it, and break-words below
never got the chance to wrap anything. #}
<dl class="grid grid-cols-[5.5rem_minmax(0,1fr)] gap-x-3 gap-y-1.5 px-4 py-3">
<dt class="text-ink-faint text-right">{{ 'message.details.from'|trans }}</dt>
<dd class="text-ink-soft break-words">
{{ details.address({ name: message.fromName, address: message.fromAddress }) }}
</dd>
{# Parsed like the rows around it, so it reads the same and copies
the same. The raw header is the fallback for one that names no
address this can parse, which is better shown than hidden. #}
{% if replyTo %}
<dt class="text-ink-faint text-right">{{ 'message.details.reply_to'|trans }}</dt>
<dd class="text-ink-soft break-words">{{ replyTo }}</dd>
<dd class="text-ink-soft break-words">
{{- replyToList is not empty ? details.addresses(replyToList) : replyTo -}}
</dd>
{% endif %}
{# Always rendered, empty or not — see the summary note above. #}
@@ -78,6 +78,13 @@ final class AddressHelperTest extends TestCase
yield 'empty' => ['', false];
yield 'null' => [null, false];
yield 'over 320 chars' => [str_repeat('a', 315) . '@example.com', false];
// Past RFC 5321's 64-character local part, and real: GitHub's reply
// addresses look like this, and refusing them left a reply to a
// GitHub notification with nobody to go to.
yield 'long local part' => ['reply+' . str_repeat('0f', 42) . '@reply.github.com', true];
yield 'long local part with dots' => [str_repeat('a', 63) . '.' . str_repeat('b', 30) . '@example.com', true];
yield 'long and broken' => [str_repeat('a', 70) . ' b@example.com', false];
}
/**
@@ -70,6 +70,22 @@ final class RecipientHeaderHelperTest extends TestCase
);
}
/**
* GitHub's reply address has a local part of about ninety characters, past
* RFC 5321's 64, and filter_var() refuses it. Dropped, a reply to any
* GitHub notification went to notifications@github.com instead, which
* accepts nothing.
*/
public function testAReplyAddressWithALongLocalPartIsKept(): void
{
$github = 'reply+017babb4f3213243ecf35c1492ef42f159c1db898253598292cf000000011822707092a169ce172b972e@reply.github.com';
self::assertSame(
[['name' => 'karatektus/sigeko', 'address' => $github]],
RecipientHeaderHelper::addresses(['reply-to' => "karatektus/sigeko <{$github}>"], 'reply-to'),
);
}
public function testAGroupWithNoMembersYieldsNobody(): void
{
self::assertSame(
@@ -0,0 +1,32 @@
<?php
declare(strict_types=1);
namespace App\Tests\Twig;
use App\Entity\Mail\Message;
use App\Twig\MessageRecipientsExtension;
use PHPUnit\Framework\TestCase;
/**
* The addresses the message header shows, for the one field with no column.
*
* Reply-To only ever lives in the stored header bag. Everywhere else the
* column is read first and the header is the fallback, so a Reply-To that fell
* through to that default would quietly show the To list as the address
* answers go to.
*/
final class MessageRecipientsExtensionTest extends TestCase
{
public function testReplyToComesFromTheHeaderRatherThanTheToColumn(): void
{
$message = new Message();
$message->toAddresses = [['name' => 'Team', 'address' => 'team@example.com']];
$message->headers = ['reply-to' => 'Tickets <tickets@example.com>'];
self::assertSame(
[['name' => 'Tickets', 'address' => 'tickets@example.com']],
new MessageRecipientsExtension()->addresses($message, 'reply-to'),
);
}
}
+53
View File
@@ -95,6 +95,59 @@ test.describe("message header recipients", () => {
});
});
test.describe("message header details layout", () => {
/**
* A long unbreakable address used to widen the whole value column past
* the panel, and the panel ran past the reading pane when a docked
* calendar narrowed it. The pane is narrowed by hand to the width that
* calendar leaves at 1100px, because which way this user's calendar switch
* is set is not something the spec can count on.
*/
test("a GitHub reply address stays inside the panel, and the panel inside the pane", async ({ page }) => {
await openThread(page, "E2E Long Reply-To");
const details = page.locator(DETAILS).first();
await details.evaluate((element) => {
let pane = element.parentElement;
while (null !== pane && "visible" === getComputedStyle(pane).overflowX && "visible" === getComputedStyle(pane).overflowY) {
pane = pane.parentElement;
}
pane!.style.maxWidth = "460px";
});
await details.getByRole("button", { name: "Details" }).click();
const panel = details.locator(PANEL);
await expect(panel).toBeVisible();
const fit = await panel.evaluate((element) => {
let pane = element.parentElement;
while (null !== pane && "visible" === getComputedStyle(pane).overflowX && "visible" === getComputedStyle(pane).overflowY) {
pane = pane.parentElement;
}
const box = element.getBoundingClientRect();
const bounds = pane!.getBoundingClientRect();
const grid = element.querySelector("dl")!;
return {
pastThePane: Math.round(Math.max(0, box.right - bounds.right, bounds.left - box.left)),
pastThePanel: grid.scrollWidth - grid.clientWidth,
};
});
expect(fit).toEqual({ pastThePane: 0, pastThePanel: 0 });
// Parsed like the rows around it, which also proves the parser took
// an address it used to refuse.
await expect(panel.getByRole("button", { name: /Copy address reply\+017babb4f/ })).toBeVisible();
});
});
test.describe("message header clipboard", () => {
test("copies one address, and shows that it did", async ({ page }) => {
await openThread(page, "E2E Read Me");