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

fix(compose): the schedule that went nowhere, and eight other things a person hit

Browse Source 
Scheduled send did nothing when a time was picked by hand, and it did it
silently. ScheduledSendResolver refuses anything closer than MIN_SECONDS (60s);
the picker only checked whether a time had PASSED, so the entire band between
now and a minute from now sailed through the browser. The server then reported
the refusal with $form->addError() — a ROOT error — and _window.html.twig
rendered only form.toAddresses's errors, so the message went nowhere at all.
Turbo replaced the compose frame with a re-render identical to what was already
on screen: menu closed, composer unchanged, no toast, one request in the
profiler, console clean.

Which is exactly the click a person makes to test the feature — they type the
next whole minute. Every green test used a PRESET; the custom picker appeared
in one test that drives it to a refusal, so "pick a time and have it actually
schedule" was asserted nowhere.

  * the picker mirrors MIN_SECONDS and refuses below it, locally and in words
    that are true of the time chosen ("past" and "too soon" are now two
    different things to be told, in three catalogues)
  * root-level form errors have somewhere to render, so no refusal from
    schedule() can be swallowed again
  * the spec drives the custom picker to a real schedule, all the way to a
    persisted badge in Drafts — and pins the floor case as a local refusal
    with no round trip at all

And the rest of the pass:

  * the signature no longer eats the first paragraph. Clicking the empty space
    BELOW it put the caret at the end of the sign-off, so typing welded onto
    it; those clicks now land in the writing space above. The emptiness check
    counted the signature as not-the-user's-writing unconditionally, so text
    that had landed inside it was subtracted before the count and Send claimed
    a visibly full message had none — the block is skipped only while it is
    untouched.
  * the subject/body confirmations are the in-app panel the plain-text switch
    already uses, not window.confirm(), which froze a test session three times.
    Both reasons are one question asked once, and the retry goes back through
    the same submitter so a scheduled send stays scheduled.
  * the To field: the refusal clears when a recipient is added, names the
    invalid address rather than blaming the count, and keeps what was typed
    (Tom Select wiped the box on blur before submit ever saw it). The
    suggestion panel no longer reopens over the Subject row on every focus.
  * aria-invalid and aria-describedby tie the refusal to the input; the From
    select and the Tom Select original are named; the picker's refusal is a
    role="alert".
  * a subject typed on its own is worth autosaving — the threshold counted the
    body alone, so "TEST-RELOAD halbfertig" was lost on reload with no warning
    and nothing in Drafts. Below the threshold, beforeunload interrupts.
  * the emoji panel is capped against the compose WINDOW rather than the
    viewport (its top edge sat 33px outside it), the grid's scrollbar is thin
    rather than a 15px native bar with arrows, and the skin-tone button is off
    the panel outline.
  * vergrößern, Schriftgröße, groß — the compose labels were the only corner
    of the UI spelling it the Swiss way, and a spec was asserting it.

phpunit OK (3427 tests, 8849 assertions); composer stan [OK] No errors;
120 compose e2e passed, plus 48 adjacent.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Paul LütznerandClaude Opus 5 committed 2026-08-13 12:56:29 +02:00
1 parent 9eec6cee64
commit ea43b1fe30
17 files changed
+1357 -44
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

assets/controllers/compose/compose_controller.js
+433 -18
View File
@@ -2,7 +2,7 @@
import { Controller } from '@hotwired/stimulus'
export default class extends Controller {
static targets = ['toField', 'ccField', 'bccField', 'subject', 'body', 'saveStatus', 'toCollection', 'collapsible', 'minimizeIcon', 'expandIcon', 'ccBtn', 'bccBtn', 'title', 'accountSelect', 'fromBtn', 'fromLabel', 'fromChevron', 'fromDropdown', 'fromRow', 'fields', 'fieldsChevron', 'fileInput', 'imageInput', 'attachments', 'scroller', 'formatBar', 'formatToggle', 'sendBtn', 'errors', 'plainBody', 'plainToggle', 'plainCheck', 'plainWarning', 'plainWarningConfirm', 'richOnly'];
static targets = ['toField', 'ccField', 'bccField', 'subject', 'body', 'saveStatus', 'toCollection', 'collapsible', 'minimizeIcon', 'expandIcon', 'ccBtn', 'bccBtn', 'title', 'accountSelect', 'fromBtn', 'fromLabel', 'fromChevron', 'fromDropdown', 'fromRow', 'fields', 'fieldsChevron', 'fileInput', 'imageInput', 'attachments', 'scroller', 'formatBar', 'formatToggle', 'sendBtn', 'errors', 'plainBody', 'plainToggle', 'plainCheck', 'plainWarning', 'plainWarningConfirm', 'sendWarning', 'sendWarningBody', 'sendWarningConfirm', 'richOnly'];
/** Below this the dock window is the whole screen — matches Tailwind's md. */
static MOBILE_QUERY = '(max-width: 767px)';
@@ -140,6 +140,30 @@ export default class extends Controller {
}
}
// Leaving with writing that has not reached the server yet.
//
// The threshold above closes the common case; this closes the rest of
// it. Between the last keystroke and the debounced save there is always
// a window in which the draft on the server is older than the one on
// screen, and a reload inside that window used to take the difference
// with it in silence — no prompt, nothing in Drafts.
//
// Deliberately the NATIVE dialog, which is the opposite of the choice
// made for the send confirmations: `beforeunload` is the only hook a
// page gets before a reload, and the browser will not show anything of
// ours in its place. The text is the browser's too; a custom string has
// been ignored by every engine for years.
this._boundBeforeUnload = (event) => {
if (false === this._hasUnsavedWriting()) {
return;
}
event.preventDefault();
event.returnValue = '';
};
window.addEventListener('beforeunload', this._boundBeforeUnload);
// Inline: the thread's reply buttons step aside while we're open.
// Cached — by the time disconnect() runs the card is already detached
// and closest() would find nothing.
@@ -166,7 +190,111 @@ export default class extends Controller {
}
input.setAttribute('aria-label', label.textContent.trim());
// The original <select> too. Tom Select leaves it in the DOM under
// `ts-hidden-accessible` — visually hidden, still in the
// accessibility tree — and the <label for> in the template stops
// reaching it once the widget is built, so it reported as an unnamed
// combobox. The audit found it as the second of two.
const original = row.querySelector('select.ts-hidden-accessible, select');
if (null !== original) {
original.setAttribute('aria-label', label.textContent.trim());
}
// The error is about the FIELD, so it has to point at the field.
// Announced-but-unassociated is how this shipped: the alert was
// read out and the combobox itself still reported aria-invalid
// null, so anyone arriving at the input afterwards got no hint that
// it was the thing being complained about. The calendar form does
// this properly; this is the compose window catching up.
input.setAttribute('aria-describedby', this._errorsId());
this._tameAddressField(this._tomSelectFor(row.querySelector('.ts-wrapper')), input);
}
this._markInvalid(this.hasErrorsTarget && false === this.errorsTarget.classList.contains('hidden'));
}
/** The id of the shared refusal line, minted once so it can be pointed at. */
_errorsId() {
if (false === this.hasErrorsTarget) {
return '';
}
this.errorsTarget.id ||= `${this.element.id || 'compose'}_address_errors`;
return this.errorsTarget.id;
}
/** Flag or unflag every address combobox as the one being complained about. */
_markInvalid(invalid) {
for (const row of this._addressRows()) {
const input = row.querySelector('.ts-control input');
if (null === input) {
continue;
}
if (true === invalid) {
input.setAttribute('aria-invalid', 'true');
} else {
input.removeAttribute('aria-invalid');
}
}
}
/**
* Stop the suggestion panel from taking the window over.
*
* Two faults, one cause. Tom Select reopens the dropdown whenever the field
* takes focus, and after Enter it refreshes it against the query that has
* just been turned into a chip. So committing an address made the panel
* spring back — covering the Subject row and the top of the body, and
* offering the contradictory pair "Add test@…" and "No results found" about
* an address that was by then already a chip sitting inches away. Escape
* dismissed it and the next focus brought it back, which left Subject
* unreachable by mouse.
*
* Typing still opens it; that is a different code path (`refreshOptions` on
* input) and it is the one the feature is actually for. What is switched off
* is opening a panel nobody asked for.
*/
_tameAddressField(select, input) {
if (null === select || true === select.plmailTamed) {
return;
}
select.plmailTamed = true;
select.settings.openOnFocus = false;
// Remember what is being typed, because by the time Send is handled it
// is already gone. Clicking the button blurs the field, and Tom Select
// empties its textbox on blur when the contents cannot become an item —
// which is precisely the case worth reporting. Reading `input.value` in
// the submit handler therefore always found an empty box, so a typed
// address was thrown away and then reported as "no recipient".
//
// Listened for on the INPUT rather than through Tom Select's own `type`
// event: that event comes out of its key handlers, so a value set any
// other way never reaches it, and "any other way" includes both a paste
// and how the tests drive the field.
input?.addEventListener('input', () => {
select.plmailTyped = input.value;
});
select.on('item_add', () => {
// The query that produced this chip is spent. Left in place it is
// what the reopened panel searches for.
select.setTextboxValue('');
select.plmailTyped = '';
select.close();
// The complaint is over the moment the thing complained about is
// fixed. It used to sit there indefinitely, pushing the Subject row
// down, until the next Send happened to clear it.
this._clearError();
});
}
disconnect() {
@@ -181,6 +309,7 @@ export default class extends Controller {
this.bodyTarget.removeEventListener('dragover', this._boundBodyDragOver);
}
window.removeEventListener('beforeunload', this._boundBeforeUnload);
window.removeEventListener('plmail:integration-attached', this._boundIntegrationAttached);
const form = this.element.querySelector('form');
form.removeEventListener('input', this._boundAutosave);
@@ -904,6 +1033,78 @@ export default class extends Controller {
);
}
/**
* A click in the empty space below the content lands in writeable text,
* never in the signature.
*
* The bug this fixes: a new message opens as `<p><br></p>` + the signature
* block, so the signature is the LAST thing in the editor and everything
* below it is the editor's own padding. A contenteditable resolves a click
* on its padding to the nearest position in the nearest child — the end of
* the signature line — so the first sentence a person wrote came out as
* "-- SIG-OUTLOOK Paul" with their text welded onto it, no separator, and
* no way to get above it with the mouse at all.
*
* `event.target === bodyTarget` is what makes this precise rather than
* clever. A click that lands ON any content — the signature text included,
* which people legitimately edit — has that element as its target and is
* left completely alone. Only a click on the editor's own box, which is by
* definition a click on empty space, is redirected.
*
* It goes UP, to the writing space above the signature, rather than opening
* a paragraph below it. Below the sign-off is not where the message goes,
* and the seeded `<p><br></p>` is already sitting there for exactly this.
*/
claimWritingSpace(event) {
if (false === this.hasBodyTarget || event.target !== this.bodyTarget) {
return;
}
const signature = this._signatureBlock();
if (null === signature) {
return;
}
this._focusWritingSpace(signature);
}
/**
* Put the caret at the end of the last writeable block above `signature`,
* making one if the body has none left.
*/
_focusWritingSpace(signature) {
let space = null;
for (const child of this.bodyTarget.children) {
if (child === signature) {
break;
}
// A quote is no more writeable than the signature is: landing at
// the end of a quoted reply is the same bug wearing a hat.
if (null === child.closest('[data-quote-wrapped], [data-quoted], blockquote')) {
space = child;
}
}
if (null === space) {
space = document.createElement('p');
space.appendChild(document.createElement('br'));
this.bodyTarget.insertBefore(space, signature);
}
const range = document.createRange();
range.selectNodeContents(space);
range.collapse(false);
const selection = window.getSelection();
selection.removeAllRanges();
selection.addRange(range);
this.bodyTarget.focus();
}
/** The signature block currently in the body, if there is one. */
_signatureBlock() {
return this.hasBodyTarget
@@ -1404,6 +1605,12 @@ export default class extends Controller {
_scheduleAutosave() {
clearTimeout(this.#autosaveTimer);
// Set BEFORE the threshold check, on purpose. Writing that is too short
// to autosave is exactly the writing that would otherwise vanish
// without trace, so it is the case the unload guard most needs to know
// about.
this._unsaved = this._contentLength() > 0;
if (false === this._worthSaving()) {
this._reportPending();
@@ -1438,7 +1645,12 @@ export default class extends Controller {
// Nothing written and nothing saved yet: closing an untouched reply
// box must not leave an empty draft behind.
if (false === allowEmpty && 0 === this._typedLength() && null === this._messageId()) {
//
// "Nothing written" means the subject as well as the body — this gate
// asked about the body alone, so a draft whose only content was a
// subject was refused here even once the threshold above had let it
// through, and the save never happened. See _contentLength().
if (false === allowEmpty && 0 === this._contentLength() && null === this._messageId()) {
return;
}
@@ -1474,6 +1686,10 @@ export default class extends Controller {
this._setStatus(this._t('saved', 'Draft saved'), 'text-success');
}
// What is on screen is now what is on the server, so leaving
// costs nothing and must not be interrupted.
this._unsaved = false;
} else {
throw new Error('Server error');
}
@@ -1482,6 +1698,11 @@ export default class extends Controller {
}
}
/** Writing on screen that the server has not been told about yet. */
_hasUnsavedWriting() {
return true === this._unsaved && this._contentLength() > 0;
}
/** The status line next to the window controls. */
_setStatus(text, className) {
if (false === this.hasSaveStatusTarget) {
@@ -1504,7 +1725,29 @@ export default class extends Controller {
return true;
}
return this._typedLength() >= this.minCharsValue;
return this._contentLength() >= this.minCharsValue;
}
/**
* Everything the user has written that a draft would keep — the subject as
* well as the body.
*
* The threshold used to look at the body alone, so a subject typed on its
* own was never worth saving no matter how long it got. A tester typed
* "TEST-RELOAD halbfertig" into Subject, waited past the autosave delay,
* reloaded, and it was gone: not in the composer, not in Drafts, and no
* warning on the way out.
*
* Counting the subject does not weaken what the threshold is FOR. Its job
* is to stop a composer that was merely opened from littering the Drafts
* list — and a composer that was merely opened has an empty subject. The
* only drafts this newly saves are ones with something deliberately typed
* in them.
*/
_contentLength() {
const subject = this.hasSubjectTarget ? this.subjectTarget.value.trim().length : 0;
return subject + this._typedLength();
}
/** Characters the user has actually written, quote excluded. */
@@ -1523,19 +1766,54 @@ export default class extends Controller {
// The last two selectors cover drafts written before buildQuotedHtml
// marked the quote with data-quoted.
//
// [data-pl-signature] is here for the same reason the quote is: it is
// not the user's writing. A new window opens with the signature already
// in the body, and counting it would put every fresh draft over
// minChars — so opening the composer and closing it again would leave
// an autosaved draft containing nothing but a sign-off.
clone.querySelectorAll(
'[data-quote-wrapped], [data-quoted], [data-pl-signature], blockquote, div[style*="border-top"], div[style*="font-size:0.85em"]',
'[data-quote-wrapped], [data-quoted], blockquote, div[style*="border-top"], div[style*="font-size:0.85em"]',
).forEach((node) => node.remove());
// The signature is dropped for the same reason the quote is: it is not
// the user's writing. A new window opens with it already in the body,
// and counting it would put every fresh draft over minChars — so
// opening the composer and closing it again would leave an autosaved
// draft containing nothing but a sign-off.
//
// ONLY WHILE IT IS UNTOUCHED, though, and that qualifier is the fix.
// Text typed into the signature block still counts, because it is still
// text the user wrote. Dropping the block unconditionally is what made
// Send insist "this message has no text" about a message the user could
// read on screen — the caret had landed in the signature (see
// claimWritingSpace) and every word they typed was being subtracted
// again before the count.
clone.querySelectorAll('[data-pl-signature]').forEach((node) => {
if (node.textContent.trim() === this._pristineSignatureText()) {
node.remove();
}
});
return clone.textContent.trim().length;
}
/**
* The current identity's signature as plain text — what an UNEDITED
* signature block in the body reads as.
*
* Text rather than HTML because the comparison has to survive the round
* trip through contenteditable, which normalises markup freely (attribute
* order, `<br>` versus `<br/>`, whitespace between blocks) without anyone
* having typed a thing. The words are what "untouched" means here.
*/
_pristineSignatureText() {
const html = this._currentSignature();
if ('' === html) {
return '';
}
const probe = document.createElement('div');
probe.innerHTML = html;
return probe.textContent.trim();
}
/**
* Below the threshold the status line counts down instead of going quiet,
* so it is obvious the draft is not being kept yet.
@@ -1545,7 +1823,7 @@ export default class extends Controller {
return;
}
const missing = this.minCharsValue - this._typedLength();
const missing = this.minCharsValue - this._contentLength();
const status = this.saveStatusTarget;
status.classList.remove('text-danger', 'text-success');
@@ -1583,6 +1861,8 @@ export default class extends Controller {
this.errorsTarget.classList.remove('hidden');
this.errorsTarget.querySelector('p').textContent = text;
this._errorsId();
this._markInvalid(true);
}
_clearError() {
@@ -1590,6 +1870,8 @@ export default class extends Controller {
this.errorsTarget.classList.add('hidden');
this.errorsTarget.querySelector('p').textContent = '';
}
this._markInvalid(false);
}
/** Move the caret into the body — where Enter on a header row leads. */
@@ -1627,7 +1909,16 @@ export default class extends Controller {
* ComposeController::send); this is only the half that can still be
* friendly about it.
*/
_guardSend() {
_guardSend(submitter) {
// What is half-typed in an address field is an address the user
// believes they have entered. Committing it here is what stops Send
// from reporting "no recipient" about a field with an address visibly
// in it — and, when the address is malformed, what makes the refusal
// say so instead of blaming the count.
if (false === this._commitPendingAddresses()) {
return false;
}
if (0 === this._recipientCount()) {
this._reportError(this._t('recipientRequired', 'Add at least one recipient before sending.'));
@@ -1636,26 +1927,146 @@ export default class extends Controller {
this._clearError();
const subject = this.hasSubjectTarget ? this.subjectTarget.value.trim() : '';
// Answered already, on the panel below. One question, one answer.
if (true === this._sendConfirmed) {
this._sendConfirmed = false;
if ('' === subject && false === window.confirm(this._t('confirmNoSubject', 'This message has no subject. Send it anyway?'))) {
return false;
return true;
}
if (0 === this._typedLength() && false === window.confirm(this._t('confirmNoBody', 'This message has no text. Send it anyway?'))) {
return false;
const missing = [];
if ('' === (this.hasSubjectTarget ? this.subjectTarget.value.trim() : 'x')) {
missing.push(this._t('confirmNoSubject', 'This message has no subject.'));
}
if (0 === this._typedLength()) {
missing.push(this._t('confirmNoBody', 'This message has no text.'));
}
if (0 === missing.length) {
return true;
}
this._openSendWarning(missing, submitter);
return false;
}
/**
* Turn anything typed-but-uncommitted in To/Cc/Bcc into a chip.
*
* @returns {boolean} false when something typed is not an address, having
* said so — and having LEFT IT IN THE FIELD. Tom Select
* drops an uncommittable string on blur, so pressing Send
* with a malformed address in the box used to throw the
* text away and then complain there were no recipients:
* two wrongs, and the user could no longer see what they
* had typed to fix it.
*/
_commitPendingAddresses() {
for (const row of this._addressRows()) {
const input = row.querySelector('.ts-control input');
const select = this._tomSelectFor(row.querySelector('.ts-wrapper'));
// The box first, then what was last typed into it — see the `type`
// handler in _tameAddressField for why the box is usually empty by
// the time we get here.
const typed = (input?.value.trim() || select?.plmailTyped?.trim()) ?? '';
if ('' === typed) {
continue;
}
if (false === this.constructor.ADDRESS.test(typed)) {
this._reportError(
this._t('invalidAddress', '"%s" is not a valid email address').replace('%s', typed),
);
// Put it back where the user left it. A refusal that also
// deletes the thing being refused leaves nothing to correct.
select?.setTextboxValue(typed);
input?.focus();
return false;
}
if (null !== select) {
select.createItem(typed, false);
select.setTextboxValue('');
select.plmailTyped = '';
}
}
return true;
}
/** The in-app replacement for the two `window.confirm()` calls. */
_openSendWarning(reasons, submitter) {
if (false === this.hasSendWarningTarget) {
return;
}
// Held so the retry submits through the SAME button. The schedule
// buttons carry `formaction`, and re-submitting without the submitter
// would quietly turn "send on Monday" into "send now".
this._pendingSubmitter = submitter ?? null;
this.sendWarningBodyTarget.textContent =
`${reasons.join(' ')} ${this._t('confirmSendAnyway', 'Send it anyway?')}`;
this.sendWarningTarget.hidden = false;
this._boundSendWarningEscape ??= (event) => {
if ('Escape' === event.key) {
event.stopPropagation();
this.cancelSendAnyway();
}
};
document.addEventListener('keydown', this._boundSendWarningEscape);
if (this.hasSendWarningConfirmTarget) {
this.sendWarningConfirmTarget.focus();
}
}
_closeSendWarning() {
if (true === this.hasSendWarningTarget) {
this.sendWarningTarget.hidden = true;
}
document.removeEventListener('keydown', this._boundSendWarningEscape);
}
/** "Send anyway" — go back through the same submit, past the question. */
confirmSendAnyway() {
const submitter = this._pendingSubmitter;
this._closeSendWarning();
this._sendConfirmed = true;
this._pendingSubmitter = null;
const form = this.element.querySelector('form');
if (null !== form) {
form.requestSubmit(submitter ?? undefined);
}
}
/** "Cancel" or Escape. Nothing is sent and nothing is lost. */
cancelSendAnyway() {
this._closeSendWarning();
this._sendConfirmed = false;
this._pendingSubmitter = null;
}
_handleSubmit(event) {
if (true === this._submitting) {
event.preventDefault();
return;
}
if (false === this._guardSend()) {
if (false === this._guardSend(event.submitter)) {
event.preventDefault();
return;
@@ -1664,6 +2075,10 @@ export default class extends Controller {
clearTimeout(this.#autosaveTimer);
this._submitting = true;
// The form is on its way to the server with everything in it. Whatever
// happens next, nothing is being abandoned.
this._unsaved = false;
// TWO in the DOM, one on screen: the send pill is rendered twice from
// one partial — `md:hidden` in the window header for phones, `hidden
// md:flex` in the action bar for the desktop — and only ever one of
assets/controllers/compose/emoji_picker_controller.js
+97 -1
View File
@@ -51,7 +51,17 @@ export default class extends Controller {
* time this runs, which is what the component needs to size its grid.
*/
build() {
if (null !== this._picker || false === this.hasPanelTarget) {
if (false === this.hasPanelTarget) {
return;
}
// Re-measured on EVERY open, not only the first. The window can be
// expanded, restored or rotated between two presses of the button, and
// a cap worked out against the height it had last time is the same bug
// in slower motion.
if (null !== this._picker) {
this._fitToWindow();
return;
}
@@ -71,6 +81,92 @@ export default class extends Controller {
this.panelTarget.appendChild(picker);
this._picker = picker;
this._styleShadow(picker);
this._fitToWindow();
}
/**
* Two corrections that can only be made from inside the shadow root.
*
* The component's root is open, which is the supported way to reach it —
* page CSS cannot select into it and neither of these is exposed as a part
* or a custom property.
*
* • The grid's scrollbar. `scrollbar-color` was themed and reached it by
* inheritance, but `scrollbar-width` was never set, so the grid kept a
* 15px native bar WITH STEPPER ARROWS inside a panel where every other
* scroller in the app is a thin overlay — themed the right colour and
* the wrong size, which reads worse than not having tried. Declared here
* rather than only on the host because something in the component's own
* sheet re-establishes it, so inheriting from outside is not enough.
* • The skin-tone button, which sat 1px off the panel's own outline and so
* appeared to be resting on it. It is given the same gutter as the rest
* of the row.
*/
_styleShadow(picker) {
const root = picker.shadowRoot;
if (null === root || undefined === root) {
return;
}
const sheet = document.createElement('style');
sheet.textContent = `
.tabpanel, .emoji-menu, [role="menu"] {
scrollbar-width: thin;
}
.skintone-button-wrapper {
margin-right: 0.375rem;
}
`;
root.appendChild(sheet);
}
/**
* Keep the panel inside the window it belongs to.
*
* It opens upward out of a compose window that is itself anchored to the
* bottom of the screen, and its height was capped against the VIEWPORT
* (`50vh`) — which knows nothing about where the window's top edge is. On a
* 900px viewport with a 512px window that put the panel's top edge 33px
* above the window's own, so an emoji panel belonging to the composer was
* drawn outside it. Still on screen, and still wrong: it reads as a
* detached menu.
*
* Measured rather than guessed because the window's height is not a
* constant — it changes when the window is expanded, and again on a phone
* where the window IS the screen. The cap is written as a custom property
* so the CSS keeps its own ceiling and this only ever lowers it.
*/
_fitToWindow() {
// After a frame, because on the first open this runs in the same tick
// as the panel being revealed and the picker being appended: the panel
// has no laid-out box yet, the measurement comes out at or below zero,
// and the cap is skipped — which looks exactly like the bug it fixes.
requestAnimationFrame(() => {
const host = this.element.closest('.compose-window');
const panel = this.hasPanelTarget ? this.panelTarget : null;
if (null === host || null === panel) {
return;
}
// The panel's bottom edge is pinned to the top of the toolbar
// (`bottom-full`), so the room it has is everything between there
// and the top of the window — less a margin, so it does not sit
// flush against the window's own edge.
const room = panel.getBoundingClientRect().bottom
- host.getBoundingClientRect().top
- 16;
if (room > 0) {
panel.style.setProperty('--emoji-max-height', `${Math.round(room)}px`);
}
});
}
_handleClick(event) {
assets/controllers/compose/schedule_controller.js
+35
View File
@@ -35,6 +35,10 @@ export default class extends Controller {
// Mirrors ScheduledSendResolver::MAX_SECONDS, itself the JMAP
// maxDelayedSend. Bounds the picker so the refusal is local.
maxDays: { type: Number, default: 30 },
// Mirrors ScheduledSendResolver::MIN_SECONDS. Without it the whole band
// between "now" and "a minute from now" passed the browser and died on
// the server — see _floor().
minSeconds: { type: Number, default: 60 },
// Translated server-side, like every other string this app writes from
// JavaScript. See _window.html.twig.
i18n: { type: Object, default: {} },
@@ -129,6 +133,20 @@ export default class extends Controller {
return;
}
// Still to come, but too soon to be a schedule. This is the band the
// whole feature used to fall down: a person testing "does send later
// work" types the NEXT WHOLE MINUTE, which is almost always under
// ScheduledSendResolver::MIN_SECONDS away. The old check only asked
// whether the time had passed, so that click sailed through the browser,
// was refused by the resolver, and came back as a root-level form error
// the compose window had nowhere to render — no toast, no message, no
// schedule. Refusing it here is what makes the common case legible.
if (chosen < this._floor()) {
this._refuse(event, this._t("scheduleTooSoon", "Pick a time at least a minute from now."));
return;
}
if (chosen > zoneHorizon(this.timezoneValue, this.maxDaysValue)) {
this._refuse(
event,
@@ -172,6 +190,23 @@ export default class extends Controller {
// ── Private ───────────────────────────────────────────────────────────
/**
* The earliest wall-clock MINUTE the server will still accept.
*
* `zoneNow` already takes the instant to read, so the floor is just "now,
* plus the minimum hold" read in the configured zone. The extra 59 seconds
* are what makes a minute-granularity field comparable to a second-
* granularity rule: at 11:43:20 the minimum instant is 11:44:20, and the
* earliest *minute* every second of which clears it is 11:45. Rounding the
* other way would put 11:44 back on the accepted side of a browser check
* and on the refused side of the server's — which is the bug this exists to
* close. Erring strict costs the user one minute; erring loose costs them
* the whole feature, silently.
*/
_floor() {
return zoneNow(this.timezoneValue, new Date(Date.now() + (this.minSecondsValue + 59) * 1000));
}
/**
* Write the chosen wall clock into the hidden field AND enable it.
*
assets/styles/app.css
+19 -2
View File
@@ -1513,9 +1513,26 @@ emoji-picker {
and re-resolves :root) carries the picker with it. */
color-scheme: inherit;
/* The other half of taming the scrollbar, and the half that was missed.
`scrollbar-color` was set on an ancestor and reached the grid inside the
shadow root on its own — both scrollbar properties INHERIT, which is what
lets a page style a scroller it cannot select. `scrollbar-width` was never
declared, so the grid kept `auto`: a 15px native bar with stepper arrows
on it, inside a panel where every other scroller in the app is a thin
overlay. Themed the right colour and the wrong size is a worse tell than
leaving it alone. */
scrollbar-width: thin;
display: block;
width: 100%;
max-height: min(24rem, 50vh);
/* `--emoji-max-height` is set by compose--emoji-picker from the room the
compose WINDOW actually has above the toolbar. 50vh alone measured the
viewport, which knows nothing about where the window's top edge is, and
put the panel 33px outside the window it belongs to. The fallback keeps
the old behaviour for any host that never sets it (the settings signature
editor renders this toolbar too). */
max-height: min(24rem, 50vh, var(--emoji-max-height, 100vh));
}
/* The panel is centred over the compose WINDOW at every width — it anchors to
@@ -1541,7 +1558,7 @@ emoji-picker {
rather than assumed — compose-mobile.spec.ts asserts the box is inside
it at 393px and at 320px. */
emoji-picker {
max-height: min(20rem, calc(100dvh - 12rem));
max-height: min(20rem, calc(100dvh - 12rem), var(--emoji-max-height, 100vh));
}
}
src/Service/Mail/ScheduledSendResolver.php
+10 -1
View File
@@ -84,11 +84,20 @@ final readonly class ScheduledSendResolver
$sendAt = $this->toInstant($submitted, $user);
// Gone, and merely too soon, are two different things to be told.
// They shared the "already passed" wording until a tester typed the
// next whole minute, was refused for a time that had plainly not
// passed, and — because the refusal had nowhere to render — was told
// nothing at all.
if ($sendAt->getTimestamp() <= $now->getTimestamp()) {
throw new InvalidScheduleException('compose.schedule.error.past');
}
// Strictly less than the floor, not less than zero. See MIN_SECONDS:
// a hold measured in seconds is a send, and answering "fine" to it
// would mean the cancel window and the hold expire together.
if ($sendAt->getTimestamp() - $now->getTimestamp() < self::MIN_SECONDS) {
throw new InvalidScheduleException('compose.schedule.error.past');
throw new InvalidScheduleException('compose.schedule.error.too_soon');
}
if ($sendAt->getTimestamp() - $now->getTimestamp() > self::MAX_SECONDS) {
templates/compose/_send_pill.html.twig
+11
View File
@@ -64,9 +64,15 @@ Variables: message, params (from the including context); placement.
an instant with the tz database in hand. #}
data-compose--schedule-timezone-value="{{ clock.timezone }}"
data-compose--schedule-max-days-value="{{ constant('App\\Service\\Mail\\ScheduledSendResolver::MAX_SECONDS') // 86400 }}"
{# The floor, from the same constant the resolver enforces. The picker used
to check only "has this passed", so every time under a minute out — which
is exactly what a person types to test the feature — passed here and was
refused there. #}
data-compose--schedule-min-seconds-value="{{ constant('App\\Service\\Mail\\ScheduledSendResolver::MIN_SECONDS') }}"
data-compose--schedule-i18n-value="{{ {
scheduleUnreadable: 'compose.schedule.error.unreadable'|trans,
schedulePast: 'compose.schedule.error.past'|trans,
scheduleTooSoon: 'compose.schedule.error.too_soon'|trans,
scheduleTooFar: 'compose.schedule.error.too_far'|trans({
'%days%': constant('App\\Service\\Mail\\ScheduledSendResolver::MAX_SECONDS') // 86400,
}),
@@ -169,7 +175,12 @@ Variables: message, params (from the including context); placement.
<p class="text-[11px] text-ink-faint">
{{ 'compose.schedule.timezone_hint'|trans({ '%zone%': clock.timezone }) }}
</p>
{# role="alert" because this is the only thing that happens when a
time is refused: the menu stays put and a line appears. Without it
a screen reader is told nothing at all, and "nothing happened" is
precisely the complaint this whole round is about. #}
<p data-compose--schedule-target="error" hidden
role="alert" aria-live="assertive"
class="text-xs text-danger"></p>
<button type="submit" formnovalidate
formaction="{{ scheduleUrl }}"
templates/compose/_window.html.twig
+78
View File
@@ -52,6 +52,7 @@ Variables: form, message, ctx (inline, frame, thread, urlParams)
recipientRequired: 'compose.recipient_required'|trans,
confirmNoSubject: 'compose.confirm.no_subject'|trans,
confirmNoBody: 'compose.confirm.no_body'|trans,
confirmSendAnyway: 'compose.confirm.send_anyway'|trans,
tooLarge: 'compose.attach_too_large'|trans({ '%name%': '%s', '%limit%': '%d' }),
imageFailed: 'compose.image_insert_failed'|trans,
noSignature: 'compose.signature_none'|trans,
@@ -213,8 +214,15 @@ Variables: form, message, ctx (inline, frame, thread, urlParams)
line is a custom menu built from these options, and this is only
the value the form posts. A widget here would be one the user
cannot see and the screen reader would announce twice. #}
{# `sr-only` is not `hidden`: this IS exposed, deliberately, so the
From value is reachable without the custom menu above. Exposed
and unnamed is the one combination that helps nobody, and it is
what shipped — the audit found it as one of exactly two unnamed
controls in the window. It borrows the visible From label rather
than inventing a second wording. #}
<select id="{{ form.account.vars.id }}"
name="{{ form.account.vars.full_name }}"
aria-labelledby="{{ form.vars.id }}_from_label"
data-compose--compose-target="accountSelect"
class="sr-only">
{% for choice in form.account.vars.choices %}
@@ -232,6 +240,27 @@ Variables: form, message, ctx (inline, frame, thread, urlParams)
data-compose--compose-toolbar-target="hiddenInput"
value="{{ form.bodyHtml.vars.value|e('html_attr') ?: '' }}">
{# ── Refusals that belong to the message, not to one field ──
This block did not exist, and its absence is what made scheduled
send look inert. ComposeController::schedule() reports a time it
will not accept with `$form->addError()` — a ROOT error — and the
only errors this template rendered were `form.toAddresses`'s. So
the refusal was written into a void: Turbo replaced the compose
frame with a re-render that looked identical to what was already
on screen, and the user saw a menu close and nothing else happen.
Outside `fields` on purpose: that block is folded away in the
inline composer, and a refusal nobody can see is the bug being
fixed here, not a shape to reproduce. #}
{% if form.vars.errors|length > 0 %}
<div class="px-4 py-2 shrink-0" role="alert" aria-live="assertive"
data-compose--compose-target="formErrors">
{% for error in form.vars.errors %}
<p class="text-xs text-danger">{{ error.message }}</p>
{% endfor %}
</div>
{% endif %}
{# Addresses, subject and the editor scroll as one region rather
than the editor scrolling under fixed field rows. On a phone
with the keyboard up there is no room for both, and Gmail
@@ -414,6 +443,11 @@ Variables: form, message, ctx (inline, frame, thread, urlParams)
<div id="{{ form.bodyHtml.vars.id }}_editor"
data-compose--compose-toolbar-target="editor"
data-compose--compose-target="body"
{# Clicking the empty area BELOW the signature used to put
the caret at the end of the signature line, so the first
thing typed in a fresh message was concatenated onto the
sign-off. See compose--compose#claimWritingSpace. #}
data-action="click->compose--compose#claimWritingSpace"
contenteditable="true"
role="textbox"
aria-multiline="true"
@@ -503,6 +537,50 @@ Variables: form, message, ctx (inline, frame, thread, urlParams)
</div>
</div>
{# ── Send-anyway warning ──────────────────────────────
The subject/body confirmations, which were `window.confirm()`
until a tester's session froze on them three times. A native
dialog blocks the whole tab, cannot be themed, and — the part
that actually matters — was the SECOND kind of confirmation in
one window, sitting a few centimetres from the in-app panel
above that asks the same sort of question. This is that panel,
with its text filled in by the controller.
One panel for both questions rather than two in sequence: a
message with neither subject nor text asked twice, and the
second dialog appeared after the first had been dismissed with
no way back. The reasons are listed together and answered
once. #}
<div data-compose--compose-target="sendWarning" hidden
role="alertdialog"
aria-labelledby="{{ form.vars.id }}_send_warning_title"
aria-describedby="{{ form.vars.id }}_send_warning_body"
class="absolute bottom-full mb-2 left-2 right-2 md:left-auto md:right-3 md:w-80
z-40 popover p-3 text-sm">
<p id="{{ form.vars.id }}_send_warning_title"
class="text-sm font-semibold text-ink">
{{ 'compose.send_warning.title'|trans }}
</p>
<p id="{{ form.vars.id }}_send_warning_body"
data-compose--compose-target="sendWarningBody"
class="mt-1 text-xs text-ink-muted"></p>
<div class="mt-2.5 flex items-center gap-2">
<button type="button"
data-compose--compose-target="sendWarningConfirm"
data-action="click->compose--compose#confirmSendAnyway"
class="rounded-full bg-accent text-white text-sm font-semibold
px-3 py-1.5 hover:bg-white/10 transition-colors">
{{ 'compose.send_warning.continue'|trans }}
</button>
<button type="button"
data-action="click->compose--compose#cancelSendAnyway"
class="rounded-full px-3 py-1.5 text-sm text-ink-muted
hover:bg-hover hover:text-ink transition-colors">
{{ 'compose.send_warning.cancel'|trans }}
</button>
</div>
</div>
{# Wraps onto a second row rather than scrolling sideways.
Scrolling hid controls behind a gesture nobody performs, and
its `overflow-x: auto` computed `overflow-y: auto` with it,
tests/Service/Mail/ScheduledSendResolverTest.php
+23 -1
View File
@@ -103,8 +103,15 @@ final class ScheduledSendResolverTest extends TestCase
/**
* And so is a time so close that the hold and the cancel window would
* expire together — see ScheduledSendResolver::MIN_SECONDS.
*
* Refused with its OWN reason, which is the point of the test. The floor
* used to answer "that time has already passed" about a time that plainly
* had not, and that wording is what made the refusal impossible to act on:
* a person types the next whole minute — almost always under the floor —
* and is told something they can see is untrue. The two conditions are two
* different things to be told.
*/
public function testATimeSecondsAwayIsRefusedAsWellAsOneAlreadyGone(): void
public function testATimeSecondsAwayIsRefusedAsTooSoonRatherThanAsPast(): void
{
$now = new DateTimeImmutable('2026-08-10T06:00:00', new DateTimeZone('UTC'));
@@ -113,6 +120,21 @@ final class ScheduledSendResolverTest extends TestCase
$this->resolver(self::BERLIN)->resolve('2026-08-10T08:00:30', $this->user(), $now);
self::fail('a schedule inside the floor should have been refused');
} catch (InvalidScheduleException $refusal) {
self::assertSame('compose.schedule.error.too_soon', $refusal->translationKey);
}
}
/** A time genuinely behind the clock still says so. */
public function testATimeAlreadyGoneIsRefusedAsPast(): void
{
$now = new DateTimeImmutable('2026-08-10T06:00:00', new DateTimeZone('UTC'));
try {
// 07:59 Berlin is 05:59 UTC — a minute behind.
$this->resolver(self::BERLIN)->resolve('2026-08-10T07:59', $this->user(), $now);
self::fail('a schedule in the past should have been refused');
} catch (InvalidScheduleException $refusal) {
self::assertSame('compose.schedule.error.past', $refusal->translationKey);
}
tests/e2e/compose-formatting.spec.ts
+67
View File
@@ -152,6 +152,73 @@ test.describe("emoji", () => {
await expect(page.locator(`${DOCK} ${PICKER}`)).toBeVisible();
}
/**
* The panel belongs to the compose window, so it stays inside it.
*
* It opens upward out of a window already anchored to the bottom of the
* screen, and its height was capped against the VIEWPORT — which knows
* nothing about where the window's top edge is. On a 1280×900 desktop that
* put the panel's top edge 33px ABOVE the window's own: still on screen,
* and reading as a detached menu belonging to nothing.
*
* Measured against the window rather than the viewport, because "inside the
* viewport" was true the whole time it was wrong.
*/
test("the panel stays inside the compose window it belongs to", async ({ page }) => {
await openCompose(page);
await openPicker(page);
const panel = page.locator(`${DOCK} .emoji-panel`);
const window_ = page.locator(`${DOCK} .compose-window`);
const panelBox = (await panel.boundingBox())!;
const windowBox = (await window_.boundingBox())!;
expect(
panelBox.y,
`the panel's top is inside the window: ${JSON.stringify({ panelBox, windowBox })}`,
).toBeGreaterThanOrEqual(windowBox.y - 0.5);
expect(panelBox.y + panelBox.height).toBeLessThanOrEqual(
windowBox.y + windowBox.height + 0.5,
);
// Still worth opening — a panel capped to nothing is inside the window
// too, which is how the containment could be "fixed" without fixing it.
expect(panelBox.height, JSON.stringify(panelBox)).toBeGreaterThan(150);
});
/**
* The two shadow-root corrections, which page CSS cannot make and so were
* both missed: a themed scrollbar that kept its 15px native width with
* stepper arrows, and the skin-tone button sitting 1px off the panel's
* outline so that it appeared to rest on it.
*/
test("the grid's scrollbar is thin, and the skin-tone button is off the edge", async ({
page,
}) => {
await openCompose(page);
await openPicker(page);
const measured = await page.locator(`${DOCK} ${PICKER}`).evaluate((host) => {
const root = (host as HTMLElement & { shadowRoot: ShadowRoot }).shadowRoot;
const grid = root.querySelector(".tabpanel")!;
const skin = root.querySelector(".skintone-button-wrapper")!;
return {
scrollbarWidth: getComputedStyle(grid).scrollbarWidth,
clearance:
host.getBoundingClientRect().right - skin.getBoundingClientRect().right,
};
});
expect(measured.scrollbarWidth).toBe("thin");
expect(
measured.clearance,
"the skin-tone button is not resting on the panel outline",
).toBeGreaterThan(3);
});
test("a chosen emoji lands in the body at the cursor", async ({ page }) => {
await openCompose(page);
tests/e2e/compose-recipients.spec.ts
+157
View File
@@ -142,3 +142,160 @@ test.describe("compose recipients", () => {
await expect(page.locator(`${DOCK} [data-compose--compose-toolbar-target="editor"]`)).toBeVisible();
});
});
/**
* What the To field does when it is unhappy, and what it does once it is happy
* again.
*
* All of these came out of a minute of manual use, and none were things the
* tests above could see: they assert what is COMMITTED, and these are about
* what is SHOWN — a message that outlives its cause, a message that names the
* wrong cause, text thrown away without being mentioned, and a suggestion panel
* that reopens over the rest of the form.
*/
test.describe("the To field's feedback", () => {
const ERRORS = `${DOCK} [data-compose--compose-target="errors"]`;
function toInput(page: Page) {
return page.locator(`${DOCK} .ts-control input`).first();
}
/**
* The complaint ends when the thing complained about is fixed.
*
* It used to stay indefinitely — cleared only by the next Send attempt —
* holding the Subject row about 19px lower than it belongs the whole time.
*/
test("the no-recipient message clears as soon as a recipient is added", async ({ page }) => {
await openCompose(page);
await page.locator(DOCK).getByRole("button", { name: "Send", exact: true }).click();
const errors = page.locator(ERRORS);
await expect(errors).toBeVisible();
await expect(errors).toContainText(/at least one recipient/i);
// The row it displaces is the point of the complaint, so measure the
// displacement itself rather than the Subject row's absolute position —
// committing a chip grows the To row by about as much as the message
// shrinks it, and the two cancel out to no movement at all.
const pushed = (await errors.boundingBox())!.height;
expect(pushed, "the message takes up room while it is shown").toBeGreaterThan(0);
await addRecipient(page, KNOWN);
await expect(errors).toBeHidden();
await expect(chips(page)).toHaveCount(1);
expect(await errors.boundingBox(), "and gives it all back").toBeNull();
});
/**
* Send with a malformed address in the box: say what is actually wrong, and
* KEEP THE TEXT.
*
* It used to report "add at least one recipient" — true of the committed
* chips, useless to someone looking at an address they had just typed — and
* Tom Select then dropped the uncommittable string on blur, so the thing
* that needed fixing vanished while being complained about.
*/
test("Send on an invalid address says so, and does not throw the text away", async ({
page,
}) => {
await openCompose(page);
await toInput(page).fill("keine-gueltige-adresse");
await page.locator(DOCK).getByRole("button", { name: "Send", exact: true }).click();
const errors = page.locator(ERRORS);
await expect(errors).toBeVisible();
await expect(errors).toContainText(/not a valid email address/i);
await expect(errors).not.toContainText(/at least one recipient/i);
// Still there to be corrected.
await expect(toInput(page)).toHaveValue("keine-gueltige-adresse");
await expect(chips(page)).toHaveCount(0);
});
/**
* A valid address typed but never confirmed is still an address the user
* entered. Send commits it rather than reporting an empty field.
*/
test("Send commits an address that was typed but not confirmed", async ({ page }) => {
await openCompose(page);
await toInput(page).fill(OTHER);
await page
.locator(`${DOCK} [data-compose--compose-target="subject"]`)
.fill("Uncommitted recipient");
await page
.locator(`${DOCK} [data-compose--compose-toolbar-target="editor"]`)
.fill("Body text long enough to matter.");
await page.locator(DOCK).getByRole("button", { name: "Send", exact: true }).click();
await expect(page.locator(ERRORS)).toBeHidden();
await expect(page.locator(`${DOCK} .compose-window`)).toHaveCount(0, { timeout: 10_000 });
});
/**
* The suggestion panel does not reopen over the rest of the form.
*
* After Enter it used to spring back — covering the Subject row and the top
* of the body — and to return on every subsequent focus, so Subject was
* repeatedly unreachable with a mouse.
*/
test("the suggestion panel stays shut after a chip is committed", async ({ page }) => {
await openCompose(page);
const dropdown = page.locator(`${DOCK} .ts-dropdown`).first();
await addRecipient(page, KNOWN);
await expect(chips(page)).toHaveCount(1);
await expect(dropdown).toBeHidden();
// And it stays shut when focus comes back — what made Subject
// unreachable was the reopening, not the first opening.
await toInput(page).click();
await expect(dropdown).toBeHidden();
// Measured rather than inferred: the Subject row can actually be
// clicked, which is the complaint in its original form.
const subject = page.locator(`${DOCK} [data-compose--compose-target="subject"]`);
await subject.click();
await expect(subject).toBeFocused();
});
/**
* The refusal is tied to the field it is about.
*
* It was announced (`role="alert"`) and otherwise orphaned: the combobox
* reported `aria-invalid` null and `aria-describedby` null, so anyone
* arriving at the input afterwards had no way to know it was the thing
* being complained about. The calendar form does this properly; compose was
* the outlier.
*/
test("the refusal is wired to the input, and unwired again when it clears", async ({
page,
}) => {
await openCompose(page);
const input = toInput(page);
const errors = page.locator(ERRORS);
await page.locator(DOCK).getByRole("button", { name: "Send", exact: true }).click();
await expect(errors).toBeVisible();
await expect(input).toHaveAttribute("aria-invalid", "true");
const describedBy = await input.getAttribute("aria-describedby");
expect(describedBy, "the input points at the message").toBeTruthy();
expect(await errors.getAttribute("id")).toBe(describedBy);
await addRecipient(page, KNOWN);
await expect(errors).toBeHidden();
await expect(input).not.toHaveAttribute("aria-invalid", "true");
});
});
tests/e2e/compose-safety.spec.ts
+88 -7
View File
@@ -178,7 +178,59 @@ test.describe("compose safety", () => {
expect(status.text).toMatch(/recipient|Empfänger/i);
});
test("an empty subject asks before sending", async ({ page }) => {
/**
* The question is asked IN THE APP, not by the browser.
*
* It was `window.confirm()`, which froze a tester's session three times: a
* native dialog blocks the whole tab, takes no styling, and sat a few
* centimetres from the in-app panel this window already uses to ask whether
* to remove formatting. Two kinds of confirmation for two versions of one
* question.
*
* The old version of this test dismissed a `dialog` event, which is why it
* would keep passing whether the dialog appeared or not — nothing was sent
* either way. It now names the panel, and fails if a native dialog is what
* comes back.
*/
test("an empty subject asks before sending, in the app rather than in a browser dialog", async ({
page,
}) => {
const sends = watchSends(page);
const native: string[] = [];
page.on("dialog", (dialog) => {
native.push(dialog.message());
void dialog.dismiss();
});
await openCompose(page);
await toInput(page).fill(VALID);
await toInput(page).press("Enter");
await page
.locator(`${DOCK} [data-compose--compose-toolbar-target="editor"]`)
.fill("Body but no subject line.");
await page.locator(DOCK).getByRole("button", { name: "Send", exact: true }).click();
const panel = page.locator(`${DOCK} [data-compose--compose-target="sendWarning"]`);
await expect(panel).toBeVisible();
await expect(panel).toHaveAttribute("role", "alertdialog");
await expect(panel).toContainText(/no subject/i);
expect(native, "the browser must not be the one asking").toHaveLength(0);
// Declining sends nothing and keeps the window as it was.
await panel.getByRole("button", { name: "Keep editing" }).click();
await expect(panel).toBeHidden();
await page.waitForTimeout(1_000);
expect(sends).toHaveLength(0);
await expect(page.locator(`${DOCK} .compose-window`)).toBeVisible();
});
/** And accepting it actually sends — the panel is a gate, not a wall. */
test("the send-anyway panel lets the message through when it is accepted", async ({ page }) => {
const sends = watchSends(page);
await openCompose(page);
@@ -189,12 +241,37 @@ test.describe("compose safety", () => {
.locator(`${DOCK} [data-compose--compose-toolbar-target="editor"]`)
.fill("Body but no subject line.");
// Refuse the confirm: nothing may go out.
page.once("dialog", (dialog) => dialog.dismiss());
await page.locator(DOCK).getByRole("button", { name: "Send", exact: true }).click();
await page.waitForTimeout(1_000);
expect(sends).toHaveLength(0);
const panel = page.locator(`${DOCK} [data-compose--compose-target="sendWarning"]`);
await expect(panel).toBeVisible();
await panel.getByRole("button", { name: "Send anyway" }).click();
await expect(page.locator(`${DOCK} .compose-window`)).toHaveCount(0, { timeout: 10_000 });
expect(sends.length, "the accepted send goes out").toBeGreaterThan(0);
});
/**
* Both reasons at once are one question, asked once.
*
* Sequential `window.confirm()`s meant a message with neither subject nor
* body asked twice, and the second dialog arrived after the first had been
* answered with no way back to it.
*/
test("a message missing both subject and text asks once, naming both", async ({ page }) => {
await openCompose(page);
await toInput(page).fill(VALID);
await toInput(page).press("Enter");
await page.locator(DOCK).getByRole("button", { name: "Send", exact: true }).click();
const panel = page.locator(`${DOCK} [data-compose--compose-target="sendWarning"]`);
await expect(panel).toBeVisible();
await expect(panel).toContainText(/no subject/i);
await expect(panel).toContainText(/no text/i);
});
/**
@@ -375,11 +452,15 @@ test.describe("compose localization", () => {
// The dock chrome and the formatting toolbar too (I-03).
for (const name of [
"Minimieren",
"Fenster vergrössern",
// ß, not Swiss ss. The compose labels were the only corner of the
// UI spelling it "vergrössern" — and this list is why it survived
// an audit: the spec asserted the misspelling, so the catalogue and
// the test agreed with each other and with nothing else.
"Fenster vergrößern",
"Linksbündig ausrichten",
"Nummerierte Liste",
"Rückgängig",
"Einzug vergrössern",
"Einzug vergrößern",
]) {
await expect(
dock.getByRole("button", { name, exact: true }),
tests/e2e/compose-scheduled-send.spec.ts
+133
View File
@@ -53,6 +53,24 @@ async function fillDock(page: Page, subject = SUBJECT): Promise<void> {
* schedule one, and "the visible Send options button" is a different element at
* 393px than at 1280px. Everything below names which one it means.
*/
/**
* A wall clock `seconds` from now, in the spelling `datetime-local` uses.
*
* The browser's own zone, deliberately: these tests run with the container's
* timezone as the configured one, so the two agree, and the point of the
* exercise is the distance from now rather than the zone arithmetic (which
* ScheduledSendResolverTest covers against real instants).
*/
function wallClockIn(seconds: number): string {
const at = new Date(Date.now() + seconds * 1000);
const pad = (value: number) => String(value).padStart(2, "0");
return (
`${at.getFullYear()}-${pad(at.getMonth() + 1)}-${pad(at.getDate())}` +
`T${pad(at.getHours())}:${pad(at.getMinutes())}`
);
}
function pill(page: Page, placement: "bar" | "header" = "bar") {
return page.locator(`${DOCK} [data-compose-send-pill="${placement}"]`);
}
@@ -305,6 +323,121 @@ test.describe("scheduled send", () => {
await expect(page.locator(`${DOCK} .compose-window`)).toBeVisible();
await expect(page.locator("#toast-region").getByText(/Scheduled for/)).toHaveCount(0);
});
/**
* THE PATH A PERSON ACTUALLY TAKES, and the one this file used to leave
* untested.
*
* Every passing test above set the hold from a PRESET. The custom picker
* appeared only in the test above this one, which drives it to a REFUSAL —
* so "pick a date and time, and have the message actually be scheduled" was
* asserted nowhere, and the feature was broken in exactly that gap while the
* suite stayed green.
*
* The time is minutes out rather than tomorrow because that is what a human
* types, and because the interesting bugs live near the floor rather than
* near the ceiling.
*/
test("a custom date and time schedules the send, all the way to the Drafts list", async ({
page,
}) => {
const subject = "Scheduled from the custom picker";
await openCompose(page);
await fillDock(page, subject);
const options = await openSendOptions(page);
await options.getByText("Pick date & time").click();
const field = options.locator('input[type="datetime-local"]');
await expect(field).toBeVisible();
await field.fill(wallClockIn(10 * 60));
await options.getByRole("button", { name: "Schedule send" }).click();
// The same three consequences a preset has. Not "a request was made":
// the broken version made a request too, and that is precisely why the
// absence of this assertion cost nothing to break.
await expect(page.locator("#toast-region").getByText(/Scheduled for/)).toBeVisible({
timeout: 10_000,
});
await expect(page.locator(`${DOCK} .compose-window`)).toHaveCount(0);
await page.goto("/mail/drafts");
const row = page
.locator('#message-list li[data-controller="mail--message-row"]')
.filter({ hasText: subject })
.first();
await expect(row).toBeVisible();
await expect(row.locator("[data-scheduled-badge]")).toBeVisible();
// Server-side, not merely on screen — a badge painted by a Turbo stream
// and never persisted would pass everything above.
await page.reload();
await expect(
page
.locator('#message-list li[data-controller="mail--message-row"]')
.filter({ hasText: subject })
.first()
.locator("[data-scheduled-badge]"),
).toBeVisible();
});
/**
* The floor, refused in the browser rather than swallowed by the server.
*
* This is the exact click that was reported as "Schedule send does
* nothing": subject and body present, the next whole minute typed into the
* picker, Schedule send pressed. Under ScheduledSendResolver::MIN_SECONDS
* the server refused it, reported the refusal as a ROOT form error, and
* re-rendered a compose window that had nowhere to render one — so the menu
* closed, the composer sat there unchanged, no toast appeared, and nothing
* was ever scheduled or sent. A request WAS made, which is what made it look
* like a server bug rather than a missing message.
*
* Both halves are asserted: the refusal is now local and legible, and
* nothing is posted at all.
*/
test("a time inside the one-minute floor is refused in the window, not silently", async ({
page,
}) => {
await openCompose(page);
await fillDock(page, "Scheduled too soon");
const posts: string[] = [];
page.on("request", (request) => {
if ("POST" === request.method() && request.url().includes("/compose/schedule")) {
posts.push(request.url());
}
});
const options = await openSendOptions(page);
await options.getByText("Pick date & time").click();
const field = options.locator('input[type="datetime-local"]');
await expect(field).toBeVisible();
// Forty seconds out, which is what "the next whole minute" usually is.
await field.fill(wallClockIn(40));
await options.getByRole("button", { name: "Schedule send" }).click();
// Said, in the menu, in words that are true of the time chosen — the old
// wording claimed it had already passed, which it plainly had not.
const refusal = options.locator('[data-compose--schedule-target="error"]');
await expect(refusal).toBeVisible();
await expect(refusal).toHaveText(/at least a minute/);
// Announced, too. The refusal IS the entire response to the click.
await expect(refusal).toHaveAttribute("role", "alert");
// The window stays, with everything still in it, and nothing went out.
await expect(page.locator(`${DOCK} .compose-window`)).toBeVisible();
await expect(page.locator("#toast-region").getByText(/Scheduled for/)).toHaveCount(0);
expect(posts, "the refusal is local — no round trip at all").toHaveLength(0);
});
});
/**
tests/e2e/compose-signature-and-options.spec.ts
+95
View File
@@ -100,6 +100,101 @@ async function clearAccountSignature(page: Page): Promise<void> {
await expect(page.getByText("Saved.")).toBeVisible();
}
test.describe("writing above the signature", () => {
/**
* The reported bug, and it is a mouse bug: clicking the empty white area
* BELOW the signature put the caret at the end of the signature LINE, so
* the first thing typed in a new message came out welded onto the sign-off
* — "-- SIG-OUTLOOK Paul" followed immediately by the sentence, no
* separator — and there was no way with a mouse to start a paragraph above
* it at all.
*
* A contenteditable resolves a click on its own padding to the nearest
* position in the nearest child, and a new body ends with the signature, so
* every click in the empty space below it landed there. The window seeds a
* `<p><br></p>` writing space ABOVE the signature; the fix sends those
* clicks to it.
*/
test("clicking below the signature types above it, not onto it", async ({ page }) => {
const SIG = "SIG-OUTLOOK Paul";
await setAccountSignature(page, SIG);
try {
await openCompose(page);
const editor = page.locator(`${DOCK} ${EDITOR}`);
await expect(editor.locator(SIGNATURE)).toBeVisible();
// The empty area below the signature — a real click at a real
// point, low in the editor's box, which is what a person does.
const box = (await editor.boundingBox())!;
await page.mouse.click(box.x + box.width / 2, box.y + box.height - 8);
await page.keyboard.type("Diese Mail geht an beide Konten.");
const signature = editor.locator(SIGNATURE);
// The signature still says only what the signature said. This is
// the assertion the bug fails: it used to read
// "SIG-OUTLOOK PaulDiese Mail geht an beide Konten."
await expect(signature).toHaveText(new RegExp(`^\\s*${SIG}\\s*$`));
// And the typing is in the body, above it.
await expect(editor).toContainText("Diese Mail geht an beide Konten.");
const typedIsAbove = await editor.evaluate((node) => {
const sig = node.querySelector("[data-pl-signature]")!;
const text = [...node.childNodes].find(
(child) => child.textContent?.includes("Diese Mail geht an beide Konten."),
)!;
return sig.compareDocumentPosition(text) === Node.DOCUMENT_POSITION_PRECEDING;
});
expect(typedIsAbove, "the paragraph is above the sign-off").toBe(true);
// …and Send does not then claim the message is empty. The emptiness
// check subtracted the whole signature block, so text that had
// landed inside it was subtracted with it and a visibly non-empty
// message was reported as having no text.
await page.locator(`${DOCK} .ts-control input`).first().fill("sig-caret@example.test");
await page.locator(`${DOCK} .ts-control input`).first().press("Enter");
await page
.locator(`${DOCK} [data-compose--compose-target="subject"]`)
.fill("Above the signature");
await page.locator(DOCK).getByRole("button", { name: "Send", exact: true }).click();
await expect(
page.locator(`${DOCK} [data-compose--compose-target="sendWarning"]`),
).toBeHidden();
} finally {
await clearAccountSignature(page);
}
});
/** Clicking ON the signature still edits the signature. */
test("clicking the signature itself still puts the caret in it", async ({ page }) => {
await setAccountSignature(page, "SIG-OUTLOOK Paul");
try {
await openCompose(page);
const editor = page.locator(`${DOCK} ${EDITOR}`);
const signature = editor.locator(SIGNATURE);
await expect(signature).toBeVisible();
await signature.click();
await page.keyboard.type("!");
await expect(signature).toContainText("!");
} finally {
await clearAccountSignature(page);
}
});
});
test.describe("compose signature", () => {
/**
* The one rule of the From switch. A signature that follows the sender is
tests/e2e/compose.spec.ts
+79
View File
@@ -189,3 +189,82 @@ test.describe("compose window", () => {
await expect(page.locator(dock)).toContainText(RECIPIENT);
});
});
/**
* Typed and then walked away from.
*
* The reported loss: "TEST-RELOAD halbfertig" typed into the SUBJECT only, six
* seconds' wait, reload — and the composer did not reopen, nothing warned, and
* the text was not in Drafts. The autosave threshold counted the BODY only, so
* a subject on its own was never worth saving however long it got, and the
* status line sat there saying "5 more characters to save" while the user typed
* twenty-two of them.
*
* The threshold still exists and still does its job — a composer that was
* merely opened has an empty subject and is still not saved. What changed is
* that it counts everything the user wrote.
*/
test.describe("work in progress is not lost", () => {
test("a subject typed on its own is autosaved, and survives a reload", async ({ page }) => {
const subject = "TEST-RELOAD halbfertig";
await page.goto("/mail/inbox");
await page.getByRole("link", { name: "Compose" }).first().click();
const dockEl = page.locator(dock);
await expect(dockEl.locator(".ts-control").first()).toBeVisible();
await dockEl.locator('[data-compose--compose-target="subject"]').fill(subject);
// The save the old threshold never made.
await page.waitForResponse(
(r) => r.url().includes("/compose/draft") && "POST" === r.request().method(),
{ timeout: 15_000 },
);
// The countdown must not still be claiming the draft is unsaved.
await expect(
dockEl.locator('[data-compose--compose-target="saveStatus"]'),
).not.toContainText(/to save/);
await page.goto("/mail/drafts");
await expect(
page
.locator('#message-list li[data-controller="mail--message-row"]')
.filter({ hasText: subject })
.first(),
).toBeVisible();
});
/**
* And below the threshold — where there is genuinely nothing on the server
* yet — leaving is interrupted rather than silent.
*
* `beforeunload` is deliberately the NATIVE dialog here, which is the
* opposite of the choice made for the send confirmations: it is the only
* hook a page gets before a reload, and no engine will show anything of
* ours in its place.
*/
test("leaving with writing too short to have saved is not silent", async ({ page }) => {
await page.goto("/mail/inbox");
await page.getByRole("link", { name: "Compose" }).first().click();
const dockEl = page.locator(dock);
await expect(dockEl.locator(".ts-control").first()).toBeVisible();
// Under the threshold on purpose: this is the residual case the
// threshold cannot cover.
await dockEl.locator('[data-compose--compose-target="subject"]').fill("ab");
const asked = await page.evaluate(() => {
const event = new Event("beforeunload", { cancelable: true });
window.dispatchEvent(event);
return event.defaultPrevented;
});
expect(asked, "the page objects to being left").toBe(true);
});
});
translations/messages.de.yaml
+16 -10
View File
@@ -1072,8 +1072,8 @@ compose:
sans: Serifenlos
serif: Serif
font_size:
huge: Sehr gross
label: Schriftgrösse
huge: Sehr groß
label: Schriftgröße
large: Gross
normal: Normal
small: Klein
@@ -1082,8 +1082,9 @@ compose:
left: Linksbündig ausrichten
right: Rechtsbündig ausrichten
confirm:
no_body: Diese Nachricht hat keinen Text. Trotzdem senden?
no_subject: Diese Nachricht hat keinen Betreff. Trotzdem senden?
no_body: Diese Nachricht hat keinen Text.
no_subject: Diese Nachricht hat keinen Betreff.
send_anyway: Trotzdem senden?
plain_text: Der Wechsel zu Nur-Text entfernt jede Formatierung aus dieser Nachricht. Fortfahren?
emoji:
activities: Aktivitäten
@@ -1133,6 +1134,7 @@ compose:
error:
past: Dieser Zeitpunkt liegt bereits in der Vergangenheit.
too_far: Nachrichten lassen sich höchstens %days% Tage zurückhalten.
too_soon: Wähle einen Zeitpunkt mindestens eine Minute in der Zukunft.
unreadable: Das ist kein gültiges Datum mit Uhrzeit.
heading: Später senden
monday_morning: Montagmorgen
@@ -1153,15 +1155,15 @@ compose:
upload:
failed: Hochladen fehlgeschlagen
images_only: In den Nachrichtentext lassen sich nur Bilder einfügen
over_limit: "Datei zu gross (max. %limit% MB)"
post_too_large: Upload zu gross
too_large: Datei zu gross
over_limit: "Datei zu groß (max. %limit% MB)"
post_too_large: Upload zu groß
too_large: Datei zu groß
add_bcc: Bcc-Empfänger hinzufügen
add_cc: Cc-Empfänger hinzufügen
attach: Dateien anhängen
attach_failed: Anhängen fehlgeschlagen
attach_remove_failed: Anhang konnte nicht entfernt werden
attach_too_large: "%name% ist grösser als %limit% MB"
attach_too_large: "%name% ist größer als %limit% MB"
bcc: Bcc
bold: Fett
cc: Cc
@@ -1171,13 +1173,13 @@ compose:
empty_draft: (leerer Entwurf)
encrypt: Nachricht verschlüsseln
encrypt_unavailable: Verschlüsselung ist noch nicht verfügbar.
expand: Fenster vergrössern
expand: Fenster vergrößern
formatting: Formatierung
forward: Weiterleiten
from: Von
image: Bild einfügen
image_insert_failed: Das Bild konnte nicht in die Nachricht eingefügt werden
indent: Einzug vergrössern
indent: Einzug vergrößern
insert_emoji: Emoji einfügen
insert_link: Link einfügen
invalid_address: "„%address%“ ist keine gültige E-Mail-Adresse"
@@ -1209,6 +1211,10 @@ compose:
continue: Fortfahren
title: Formatierung entfernen?
pop_out: In eigenem Fenster öffnen
send_warning:
cancel: Weiter bearbeiten
continue: Trotzdem senden
title: Diese Nachricht senden?
read_receipt: Lesebestätigung anfordern
recipient_required: Füge mindestens einen Empfänger hinzu, bevor du sendest.
redo: Wiederholen
translations/messages.en.yaml
+8 -2
View File
@@ -1055,8 +1055,9 @@ compose:
left: Align left
right: Align right
confirm:
no_body: This message has no text. Send it anyway?
no_subject: This message has no subject. Send it anyway?
no_body: This message has no text.
no_subject: This message has no subject.
send_anyway: Send it anyway?
plain_text: Switching to plain text removes all formatting from this message. Continue?
# Every label inside the emoji picker. The component ships its own English
# strings and takes a replacement object; this is that object, so the
@@ -1111,6 +1112,7 @@ compose:
error:
past: That time has already passed.
too_far: Mail can be held for at most %days% days.
too_soon: Pick a time at least a minute from now.
unreadable: That is not a date and time.
heading: Send later
monday_morning: Monday morning
@@ -1199,6 +1201,10 @@ compose:
continue: Continue
title: Remove formatting?
pop_out: Open in compose window
send_warning:
cancel: Keep editing
continue: Send anyway
title: Send this message?
read_receipt: Request read receipt
recipient_required: Add at least one recipient before sending.
redo: Redo
translations/messages.en_PI.yaml
+8 -2
View File
@@ -800,8 +800,9 @@ compose:
left: Hard to port
right: Hard to starboard
confirm:
no_body: This scroll be blank. Cast it off regardless?
no_subject: This scroll bears no heading. Cast it off regardless?
no_body: This scroll be blank.
no_subject: This scroll bears no heading.
send_anyway: Cast it off regardless?
plain_text: "Strikin' to plain text scrubs every flourish from this scroll. Carry on?"
emoji:
activities: Sport and mischief
@@ -851,6 +852,7 @@ compose:
error:
past: That hour be already astern.
too_far: "No scroll be held longer than %days% days."
too_soon: Name an hour a minute or more ahead, matey.
unreadable: That be no day nor hour I can read.
heading: Cast off later
monday_morning: Monday at first light
@@ -927,6 +929,10 @@ compose:
continue: "Aye, carry on"
title: "Strip the flourishes?"
pop_out: Open in its own cabin
send_warning:
cancel: "Belay, keep writin'"
continue: "Cast off regardless"
title: "Cast this scroll off?"
read_receipt: "Beg word it were read"
recipient_required: Name at least one soul afore ye cast off.
redo: Do it over again