Skip to content

fix(comments): stop the comment form from widening right-to-left pages - #3955

Merged
khoinguyenpham04 merged 3 commits into
mainfrom
noah/quirky-kowalevski-fcdf66
Oct 7, 2026
Merged

khoinguyenpham04 merged 3 commits into
mainfrom
noah/quirky-kowalevski-fcdf66

Conversation

@khoinguyenpham04

Copy link
Copy Markdown
Collaborator

What does this PR do?

<CommentForm> hid its spam honeypot with position:absolute;left:-9999px;top:-9999px. On right-to-left pages (<html dir="rtl">) that offset is on the scrollable side, so the page became about 10,000px wider and scrolled sideways into empty space. Firefox did this on every RTL page with the form. Chrome and Safari did it when the form or an element around it is positioned, for example with position: relative.

The honeypot is now hidden in place with a clipped 1px box (width:1px; height:1px; overflow:hidden; clip-path:inset(50%); white-space:nowrap). The input inside keeps its normal size, so bots still find and fill it, but people can't see or click it. The existing aria-hidden="true" and tabindex="-1" keep it away from screen readers and keyboard users.

Two physical properties also become logical, so they sit on the correct side in RTL:

  • <Comments> reply indent: padding-left → padding-inline-start
  • <CommentForm> separator before a signed-in commenter's email: margin-right → margin-inline-end

Follow-up to #3951, which noted the honeypot problem.

Type of change

  • Bug fix
  • Feature (requires maintainer-approved Discussion)
  • Refactor (no behavior change)
  • Translation
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Checklist

  • I have read CONTRIBUTING.md
  • pnpm typecheck passes
  • pnpm lint passes
  • pnpm test passes (or targeted tests for my change)
  • pnpm format has been run
  • I have added/updated tests for my changes (if applicable)
  • User-visible strings in the admin UI are wrapped for translation (if applicable). Do not include messages.po changes except in translation PRs — a workflow extracts catalogs on merge to main. (n/a: no admin UI changes)
  • I have added and reviewed the user-facing changeset (if this PR changes a published package)
  • New features link to an approved Discussion: https://github.com/emdash-cms/emdash/discussions/... (n/a: bug fix)
  • I have included screenshots below if this PR changes the UI

AI-generated code disclosure

  • This PR includes AI-generated code — model/tool: Claude Opus 5.5

Screenshots / test output

A blog template post in Firefox with dir="rtl", before the fix, scrolled 640px sideways. After the fix the same page is 1,280px wide and can't scroll sideways.

Blog post in Firefox with a right-to-left layout, scrolled sideways: the left half of the window is empty because the page is 11,279 pixels wide

Comments on a right-to-left page while signed in, before and after:

Right-to-left comments before the fix: the reply sits flush with its parent comment on the right, and the separator dot touches the email address

Right-to-left comments after the fix: the reply is indented from the right, and the separator dot has space on both sides

Document width in RTL on that post, signed out (scrollWidth, viewport 1280):

Chromium Firefox WebKit
Before 1280 11279 1280
After 1280 1280 1280

With a positioned ancestor around the form, the old honeypot gives 11271 in all three engines and the new one gives 1280.

  • The new e2e test in e2e/tests/public-page-styles.spec.ts fails on main with Expected: 1280, Received: 11271 and passes with this change. It positions <body> because Chromium, the only Playwright project, leaves the off-page box out of the scroll width otherwise.
  • It passed 30/30 with --repeat-each=15 on the Node target and 16/16 with --repeat-each=8 on EMDASH_E2E_TARGET=cloudflare.
  • With the fix, Playwright fill() still fills the honeypot (the input keeps a 147×22 box), and document.elementFromPoint at its centre returns the surrounding form, never the input.
  • Root pnpm build, pnpm typecheck, pnpm lint, and tests/repro/comments-labels.render.test.ts pass.

The honeypot field was hidden with left:-9999px. In right-to-left
documents that offset lies on the scrollable side of the page, so
Firefox, and Chromium and WebKit whenever the form has a positioned
ancestor, made the page about 10,000px wide. Hide the field in place
with a clipped 1px box instead; bots still see a fillable input, and
aria-hidden and tabindex=-1 keep it from people.

Also switch the reply indent and the signed-in email separator to
logical properties so they sit on the correct side in RTL.
@changeset-bot

changeset-bot Bot commented Oct 6, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 2468504

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 12 packages
Name Type
emdash Patch
@emdash-cms/cloudflare Patch
@emdash-cms/plugin-test Patch
@emdash-cms/sandbox-workerd Patch
@emdash-cms/admin Patch
@emdash-cms/auth Patch
@emdash-cms/blocks Patch
create-emdash Patch
@emdash-cms/gutenberg-to-portable-text Patch
@emdash-cms/x402 Patch
@emdash-cms/auth-atproto Patch
@emdash-cms/plugin-embeds Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@cloudflare-workers-and-pages

Copy link
Copy Markdown

🚀 Deploying Preview to Cloudflare 🚀

Preview URL: https://noah-quirky-kowalevski-fcdf66.try.emdashcms.com, https://noah-quirky-kowalevski-fcdf66-emdash-playground.emdash-cms.workers.dev (commit 2468504)

This URL reflects your latest Preview deployment

Preview Deployments by commit

Status Deployment URL Commit Updated (UTC) See this deployment's details
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://b43fc666.try.emdashcms.com, https://b43fc666-emdash-playground.emdash-cms.workers.dev 2468504 2026-10-06T22:23:53.688Z Visit the dashboard ↗

@pkg-pr-new

pkg-pr-new Bot commented Oct 6, 2026

Copy link
Copy Markdown

Open in StackBlitz

@emdash-cms/admin

npm i https://pkg.pr.new/@emdash-cms/admin@3955

@emdash-cms/auth

npm i https://pkg.pr.new/@emdash-cms/auth@3955

@emdash-cms/auth-atproto

npm i https://pkg.pr.new/@emdash-cms/auth-atproto@3955

@emdash-cms/blocks

npm i https://pkg.pr.new/@emdash-cms/blocks@3955

@emdash-cms/cloudflare

npm i https://pkg.pr.new/@emdash-cms/cloudflare@3955

@emdash-cms/contentful-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/contentful-to-portable-text@3955

emdash

npm i https://pkg.pr.new/emdash@3955

create-emdash

npm i https://pkg.pr.new/create-emdash@3955

@emdash-cms/gutenberg-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/gutenberg-to-portable-text@3955

@emdash-cms/plugin-cli

npm i https://pkg.pr.new/@emdash-cms/plugin-cli@3955

@emdash-cms/plugin-test

npm i https://pkg.pr.new/@emdash-cms/plugin-test@3955

@emdash-cms/plugin-types

npm i https://pkg.pr.new/@emdash-cms/plugin-types@3955

@emdash-cms/registry-client

npm i https://pkg.pr.new/@emdash-cms/registry-client@3955

@emdash-cms/registry-lexicons

npm i https://pkg.pr.new/@emdash-cms/registry-lexicons@3955

@emdash-cms/registry-loader

npm i https://pkg.pr.new/@emdash-cms/registry-loader@3955

@emdash-cms/registry-moderation

npm i https://pkg.pr.new/@emdash-cms/registry-moderation@3955

@emdash-cms/registry-verification

npm i https://pkg.pr.new/@emdash-cms/registry-verification@3955

upgrade-emdash

npm i https://pkg.pr.new/upgrade-emdash@3955

@emdash-cms/sandbox-workerd

npm i https://pkg.pr.new/@emdash-cms/sandbox-workerd@3955

@emdash-cms/x402

npm i https://pkg.pr.new/@emdash-cms/x402@3955

@emdash-cms/plugin-ai-moderation

npm i https://pkg.pr.new/@emdash-cms/plugin-ai-moderation@3955

@emdash-cms/plugin-atproto

npm i https://pkg.pr.new/@emdash-cms/plugin-atproto@3955

@emdash-cms/plugin-audit-log

npm i https://pkg.pr.new/@emdash-cms/plugin-audit-log@3955

@emdash-cms/plugin-color

npm i https://pkg.pr.new/@emdash-cms/plugin-color@3955

@emdash-cms/plugin-embeds

npm i https://pkg.pr.new/@emdash-cms/plugin-embeds@3955

@emdash-cms/plugin-field-kit

npm i https://pkg.pr.new/@emdash-cms/plugin-field-kit@3955

@emdash-cms/plugin-forms

npm i https://pkg.pr.new/@emdash-cms/plugin-forms@3955

@emdash-cms/plugin-webhook-notifier

npm i https://pkg.pr.new/@emdash-cms/plugin-webhook-notifier@3955

commit: 2468504

@emdashbot emdashbot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the right fix for the right problem: the off-screen honeypot in <CommentForm> legitimately caused RTL pages to overflow, and the patch replaces it with a clipped in-place box that keeps the honeypot functional while removing the overflow source. The two logical-property swaps (padding-left → padding-inline-start, margin-right → margin-inline-end) are also the correct, low-risk way to align reply indentation and the signed-in email separator for RTL.

I reviewed the diff, the full changed components, the e2e regression test, and the changeset against AGENTS.md conventions. No logic bugs, regressions, authorization, query-count, localization, or changeset issues were introduced. The e2e test is a sensible regression guard: it forces the positioned-ancestor case Chromium needs to reproduce the overflow, asserts scrollWidth === clientWidth, and leaves no state behind because Playwright isolates pages per test.

One non-blocking observation for a follow-up: packages/plugins/forms/src/astro/FormEmbed.astro still uses the same old style="position:absolute;left:-9999px;" honeypot pattern, so it likely has the same RTL overflow bug. That’s outside this PR’s scope and should be fixed separately rather than tacked on here.

LGTM.

@github-actions github-actions Bot added review/approved Approved; no new commits since and removed review/needs-review No maintainer or bot review yet labels Oct 6, 2026
@khoinguyenpham04
khoinguyenpham04 merged commit 291e0f8 into main Oct 7, 2026
46 checks passed
@khoinguyenpham04
khoinguyenpham04 deleted the noah/quirky-kowalevski-fcdf66 branch October 7, 2026 12:47
@emdashbot emdashbot Bot mentioned this pull request Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants