[WC-3556] fix(rich-text): dialog presentation, image paste/drop, list marker formatting

[gjulivan]

Pull request type

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Test related change (New E2E test, test automation, etc.)

---

Description

Groups five related Rich Text fixes plus one File Uploader fix. All changes are
non-breaking; stored content is untouched until the user makes an edit.

Rich Text — dialogs

• New "Dialog style" setting. inline (default, unchanged behaviour) anchors a
  dialog to the toolbar button it was opened from; focused centers it over a dimmed
  page and traps the keyboard inside until it closes. Adds the dialogStyle XML
  property and a shared DialogShell component that the image, video, link, help and
  confirm dialogs now render through.
• Dialogs are no longer clipped. A dialog taller than the viewport, or one inside a
  container that clips its content (e.g. a pop-up page), used to be cut off with its
  Cancel/Insert buttons unreachable. Dialogs now render in a body-level portal, stay
  inside the viewport, and scroll internally — the action buttons sit outside the scroll
  region so they stay visible.
• Media Library buttons no longer insert prematurely. Clicking a button inside the
  image dialog's Media Library content inserted the image and closed the dialog without
  Insert being pressed.

Rich Text — images

• Drag & drop and clipboard paste for images. New ImagePasteDrop extension. Files
  that are too large or are not images are rejected with a reason shown below the editor.
  Gated on "Enable default upload", whose description was updated to cover the new paths.
• Dropping a file no longer navigates the browser away from the page.
• v4 image sizes survive the v5 upgrade. Images resized in Rich Text 4 lost their
  size and rendered at their original dimensions.

Rich Text — list markers

• Bullets and numbers now follow the formatting of the list item's first character —
  size, bold, italic, colour and font family. Each item is evaluated independently, and
  nested lists follow their own first run.

  Implementation notes for reviewers: the marker format is derived, never stored. ::marker
  inherits from its <li>, but every format the user can apply lands on an inline mark two
  levels down, and CSS has no child-to-ancestor selector — so computeMarkerFormat reads
  the first inline run and publishes the result as --rt-marker-* custom properties, which
  affect only what ::marker reads and not the item's own content. Two delivery paths are
  needed and both call the same pure function: renderHTML (feeds getHTML(), copy/paste,
  initial render) and a Decoration.node plugin (ProseMirror does not re-invoke toDOM when
  only a node's content changes, so the attribute would otherwise go stale as you type).
  No node attribute is declared, so incoming marker data is dropped on parse and recomputed.

  The list gutter scales with the marker, since an enlarged marker grows leftward out of
  padding-left. The multiplier is marker-length-aware: measured in Chrome, a flat 1.5×
  clipped three- and four-digit numbers at the maximum font size, so the gutter is derived
  from the longest counter's character count (start + childCount - 1, with lower-roman
  counted by numeral length rather than digits). With no enlarged marker present the
  computed padding is byte-identical to the previous 1.5em.

  Task lists are out of scope — taskItem renders a checkbox with list-style: none and
  has no ::marker.

• Opening a page no longer marks the value as changed. Pre-existing and not
  list-specific: the editor's value-sync effect called setContent with updates enabled,
  so on every mount the editor's own serialization was written back over any stored value
  that was not already byte-identical to getHTML() — dirtying the bound attribute and
  firing the "On change" action without a user edit. Now passes { emitUpdate: false };
  that direction is external value → editor, so echoing back is never wanted. A genuine
  edit still emits through onUpdate.

  Side effect worth a look: the status bar's "Characters (HTML)" count now reflects the
  value as stored rather than the editor's re-serialization (one snapshot moved, 82 → 49).
  Arguably the more truthful number, but it will still shift on the user's first real edit.

Rich Text — toolbar

• Header toolbar item now appears in custom mode. Its name was mapped incorrectly.

File Uploader

• Action and retry buttons no longer submit the surrounding form, which caused the
  page to submit or a containing dialog to close unexpectedly.

---

What should be covered while testing?

Rich Text

• Both styleDataFormat modes (inline and class) for every item below — class
  mode emits class + data-* attributes instead of inline styles.
• Dialog style: switch between Inline and Focused for the image, video and link
  dialogs. Check keyboard focus stays inside a Focused dialog and Escape dismisses it.
• Dialog clipping: short viewport, and a Rich Text inside a pop-up page. Cancel and
  Insert must stay reachable.
• Image dialog: Media Library tab — clicking buttons inside the content should not
  insert or close; only Insert should.
• Images: drag & drop and paste, with "Enable default upload" both on and off.
  Oversized and non-image files should be rejected with a message. Open content saved in
  Rich Text 4 with resized images and confirm sizes are preserved.
• List markers: enlarge the first character of a bullet and a numbered item and check
  the marker follows size, bold, italic, colour and font. Sibling items should format
  independently, nested lists should follow their own first run, task list checkboxes
  should be untouched, and a long numbered list (100+ items) at a large size should not
  clip its numbers.
• On change: open a page with existing Rich Text content and close it without editing.
  The "On change" action must not fire and the stored value must be byte-identical.

File Uploader

• Place the widget inside a form and inside a pop-up page. Clicking a file action button
  or the retry button must not submit the form or close the dialog.

Browser note: class-mode marker formatting relies on typed attr()
(attr(data-marker-font-size px)), which is Chrome 133+ and not yet in Safari or Firefox.
This is the same support bar the widget's existing class-mode font size and text colour
already sit on, so it is not a new limitation — but class mode is worth a look in Safari
if that matters for this release. All five ::marker properties are verified in Chrome.

Tests

• Unit: markerFormat (29), ListItemMarkerFormat (14), ImagePasteDrop, ImageResize,
  ActionButton, RetryButton, ToolbarConfig, plus the load-time no-write regression in
  RichText.spec.tsx. pnpm run test in rich-text-web: 403 passing, 26 suites.
• E2E: dialog clipping (tall viewport + pop-up page), YouTube embed URL, and one
  parameterized marker case per list type in e2e/RichText.spec.js.
• CHANGELOG.md updated under [Unreleased] in both packages. No version bumps — those
  happen at release time.

[github-actions]

AI Code Review

⚠️ Approved with suggestions — low-severity items only, safe to merge

---

What was reviewed

File	Change
packages/pluggableWidgets/file-uploader-web/src/components/ActionButton.tsx	Add type="button", remove redundant role="button"
packages/pluggableWidgets/file-uploader-web/src/components/RetryButton.tsx	Add type="button"
packages/pluggableWidgets/file-uploader-web/src/components/__tests__/ActionButton.spec.tsx	New form-submission regression tests
packages/pluggableWidgets/file-uploader-web/src/components/__tests__/RetryButton.spec.tsx	New form-submission regression tests
packages/pluggableWidgets/file-uploader-web/CHANGELOG.md	Fixed entry for button type fix
packages/pluggableWidgets/rich-text-web/src/RichText.xml	New dialogStyle property, updated enableDefaultUpload description
packages/pluggableWidgets/rich-text-web/src/components/EditorContext.tsx	Add dialogStyle to context
packages/pluggableWidgets/rich-text-web/src/components/EditorWrapper.tsx	Thread dialogStyle through
packages/pluggableWidgets/rich-text-web/src/components/toolbars/components/DialogShell.tsx	New shared portal shell for all dialogs
packages/pluggableWidgets/rich-text-web/src/components/toolbars/components/ImageDialog.tsx	Remove <form>, add Enter-per-input, fix listener deps
packages/pluggableWidgets/rich-text-web/src/components/toolbars/components/VideoDialog.tsx	Move to DialogShell
packages/pluggableWidgets/rich-text-web/src/components/toolbars/components/LinkDialog.tsx	Move to DialogShell
packages/pluggableWidgets/rich-text-web/src/components/toolbars/components/HelpDialog.tsx	Move to DialogShell
packages/pluggableWidgets/rich-text-web/src/components/toolbars/components/ConfirmDialog.tsx	Move to DialogShell
packages/pluggableWidgets/rich-text-web/src/extensions/ImagePasteDrop.ts	New extension for drop/paste images
packages/pluggableWidgets/rich-text-web/src/extensions/ImageResize.ts	v4 image size compat via imageSize.ts
packages/pluggableWidgets/rich-text-web/src/extensions/ListItemMarkerFormat.ts	New extension for marker format
packages/pluggableWidgets/rich-text-web/src/utils/imageFiles.ts	Shared file validation/read helpers
packages/pluggableWidgets/rich-text-web/src/utils/imageSize.ts	New toCssLength/toHtmlDimension for v4 compat
packages/pluggableWidgets/rich-text-web/src/utils/markerFormat.ts	Marker format derivation utilities
packages/pluggableWidgets/rich-text-web/src/ui/RichText.scss	Drag-over highlight, dialog structural rules
packages/pluggableWidgets/rich-text-web/src/ui/Dialog.scss	Flex-column dialog, scroll region, overlay, z-index unification
packages/pluggableWidgets/rich-text-web/e2e/RichText.spec.js	New E2E: overflow/portal tests, marker tests, YouTube fix
Various __tests__/*.spec.tsx	New and updated unit tests
Both CHANGELOG.md files	Unreleased entries for all fixes

Skipped (out of scope): dist/, pnpm-lock.yaml, openspec/ archives, *.png snapshots

---

Findings

⚠️ Low — dialogStyle placed in Advanced instead of General

File: packages/pluggableWidgets/rich-text-web/src/RichText.xml line 223
Problem: The dialogStyle property was placed in Advanced > Advanced (next to styleDataFormat). The design doc for this change explicitly argues against this: "Advanced (next to styleDataFormat) groups rendering-format switches and would bury it" — and recommends "General > General, after enableStatusBar" because it is an end-user-visible presentation preference. This is currently the only discoverability gap for a user-facing feature.
Fix: Move the dialogStyle property into General > General after enableStatusBar per the design doc's recommendation, or document the deliberate deviation if there was a reason to change course.

---

⚠️ Low — Viewport not restored after overflow E2E test

File: packages/pluggableWidgets/rich-text-web/e2e/RichText.spec.js line 595
Problem: The test "a dialog taller than the viewport scrolls internally..." calls page.setViewportSize({ width: 1024, height: 420 }) and never restores it. The immediately following test ("a dialog opened inside a popup page...") navigates to "/" but runs under the shrunken viewport. Should the popup test add screenshot assertions or be order-dependent with another test that requires a normal viewport, this will cause flakiness.
Fix: Restore the viewport at the end of the test, or add a test.afterEach reset:

test.afterEach(async ({ page }) => {
    await page.setViewportSize({ width: 1280, height: 720 });
});

Or use Playwright's --viewport config so it resets between tests automatically.

---

⚠️ Low — insertImageFiles rejection is fire-and-forget

File: packages/pluggableWidgets/rich-text-web/src/extensions/ImagePasteDrop.ts line 199
Problem: insertImageFiles(...) returns a Promise<void> that is called without await (required because the DOM handler must return synchronously). Inside insertImageFiles, the call to ctx.insertImage(src, target) sits outside any try/catch. If the editor is destroyed during the FileReader read (navigation, component unmount), insertImage will throw and produce an unhandled promise rejection. The existing per-file try/catch only wraps the readFileAsDataUrl call.
Fix: Wrap the insert call in the same error boundary, or wrap the entire outer for loop body:

try {
    const target = Math.min(at, ctx.docSize());
    ctx.insertImage(src, target);
    at = target + 1;
} catch {
    // Editor disposed between read and insert — skip silently.
}

---

Positives

• The type="button" fix for ActionButton/RetryButton is minimal and correct; the form-submission regression tests (including the deliberate red-flag verification in tasks.md) are exactly the right coverage for this class of bug.
• Removing <form> from ImageDialog is the right structural fix — filtering by submitter would treat the symptom, and the design doc analysis of why SubmitEvent.submitter === null is a trap is precise.
• DialogShell unifies five different ad-hoc dialog shells into one component. The inline/focused split, portal escape, and height-bound approach are architecturally sound and the tradeoffs are well-documented.
• The two-path design for marker format (renderHTML + Decoration.node) is correctly motivated by ProseMirror's sameMarkup DOM-reuse behaviour — a single path would leave live <li> elements stale after typing.
• Holding dialogNode in state rather than a ref in ImageDialog is a subtle correctness detail for portal mount ordering, and the comment explaining it is clear.
• The enter/leave counter (dragDepth) for drag-over affordance correctly avoids flicker over descendant text nodes; reset to zero on drop prevents stuck highlights.
• handleDOMEvents over handleDrop/handlePaste is the right choice and the code comment explains why read-only gating would miss the navigation-away regression without it.
• Both CHANGELOG.md files have user-behaviour-only entries under [Unreleased] with no version bumps, matching the repo conventions exactly.