Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/pluggableWidgets/file-uploader-web/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),

## [Unreleased]

### Fixed

- We fixed an issue where clicking a file action button or the retry button submitted the surrounding form, causing the page to submit or a containing dialog to close unexpectedly.

### Changed

- Since version 2.5.0, removing a file with the default remove button removes the entry from the file list immediately, instead of leaving it in the list greyed out. This matches how removal already worked when custom buttons are configured. This change was missing from the 2.5.0 release notes.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-09-01
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
## Context

`ActionButton` renders the per-file action buttons in the files list (`.action-button`, e.g. the "add" / "remove" / custom list-action buttons). `RetryButton` renders the retry affordance on a failed upload. Both are plain `<button>` elements with an `onClick` handler and no `type` attribute.

HTML spec — the `type` attribute of `<button>` is an enumerated attribute whose _missing value default_ is `submit`. A submit button's activation behaviour is to submit its form owner (the nearest ancestor `<form>`, absent a `form` attribute). So inside any form, these buttons are submit buttons.

### Why `stopPropagation()` is not a defence

```
CLICK on .action-button
├─ event dispatch: capture -> target -> bubble stopPropagation() truncates THIS
└─ activation behaviour: submit the form owner only preventDefault() cancels THIS
```

The current handler:

```tsx
const onClick = useCallback(
(e: MouseEvent<HTMLButtonElement>) => {
e.stopPropagation();
action?.();
},
[action]
);
```

`stopPropagation()` exists here to stop the click reaching the `.file-entry` / dropzone handlers, which is a separate and legitimate concern. It has no effect on form submission.

### How this surfaced

The widget can be placed into another widget's content slot. The Rich Text image dialog wraps its content — including the app-developer-configured image-source slot — in a `<form onSubmit={...}>`:

```
<form> (rich-text ImageDialog)
<div class="image-dialog-entity">
<div class="widget-file-uploader">
...
<button class="action-button"> type defaults to submit -> submits the form above
```

The same happens on any page where an app developer nests the widget inside a form.

## Goals / Non-Goals

**Goals:**

- The widget's buttons perform their action only, never an implicit form submission.

**Non-Goals:**

- Changing `stopPropagation()` behaviour — still required for entry/dropzone isolation.
- Any styling, layout, or XML property change.
- Fixing the Rich Text image dialog's use of `<form>` — separate package, separate change (`fix-image-dialog-nested-submit`). Both are needed: this change protects every form the widget is placed in; that change protects the dialog from _any_ embedded widget, not just this one.

## Decisions

### Decision: `type="button"` on both components

The minimal, spec-correct fix. Neither button has any relationship to form submission; declaring `type="button"` states that. Preferred over adding `e.preventDefault()` in the handlers, which would suppress the submission as a side effect of an event handler rather than by declaring the element's kind, and would also suppress other default behaviours.

### Decision: drop `role="button"` from ActionButton

`<button>` has an implicit ARIA role of `button`. The explicit `role={"button"}` is redundant and, being adjacent to the missing `type`, reads as if it were making the element interactive — which it isn't. Removing it does not change the computed role, so it is not an accessibility change. `RetryButton` already omits it.
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
## Why

The File Uploader's file action buttons and retry button submit any enclosing `<form>` when clicked.

Per the HTML spec, a `<button>` whose `type` attribute is missing is in the Submit Button state: its activation behaviour submits its form owner. Both button components omit `type`:

- `src/components/ActionButton.tsx` — `<button role={"button"} className={...} onClick={onClick} title={title}>`
- `src/components/RetryButton.tsx` — `<button className="retry-button" disabled={...} onClick={onClick} title={...}>`

Both `onClick` handlers call `e.stopPropagation()`, which does not help. Propagation and default actions are independent: the browser runs the button's activation behaviour after event dispatch completes, whether or not propagation was stopped. Only `preventDefault()` cancels it.

This was found via the Rich Text widget, where a File Uploader placed in the image dialog's image-source content slot sits inside the dialog's `<form>`; clicking a file's add action button submitted that form, inserting the image and closing the dialog without the user pressing Insert. The same failure occurs for any form the widget is placed inside.

## What Changes

Package: `packages/pluggableWidgets/file-uploader-web`

- `src/components/ActionButton.tsx` — add `type="button"`. Also drop the redundant `role="button"`: a native `<button>` already has that role, and the explicit attribute adds nothing.
- `src/components/RetryButton.tsx` — add `type="button"`.

No other `<button>` elements exist in the package's source.

## Capabilities

### New Capabilities

- `file-uploader-action-buttons`: the widget's own buttons perform their action only and never submit an enclosing form.

### Modified Capabilities

None.

## Impact

**Files affected**:

- `src/components/ActionButton.tsx`
- `src/components/RetryButton.tsx`

**User-facing changes**:

- Placing the File Uploader inside a form-bearing container (for example the Rich Text image dialog's image-source content slot) no longer submits that form when a file action or retry button is clicked.
- No visual change, no XML property change, no API change. `role="button"` removal is not observable to assistive technology — the computed role is unchanged.

**Related**:

- `rich-text-web` change `fix-image-dialog-nested-submit` removes the `<form>` from the image dialog. That fix is independent and still needed, because the image dialog accepts arbitrary app-developer content and cannot rely on every embedded widget typing its buttons correctly. This change is still needed because an untyped button breaks every _other_ form the widget is dropped into, including app developers' own pages.

**Testing scope**:

- Clicking a file action button inside a `<form>` does not submit the form.
- Clicking the retry button inside a `<form>` does not submit the form.
- The button's own action still runs.
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
## ADDED Requirements

### Requirement: Widget buttons do not submit enclosing forms

Every `<button>` element rendered by the File Uploader widget SHALL declare `type="button"`. Activating a file action button or the retry button SHALL run only that button's own action and SHALL NOT cause submission of any enclosing `<form>`, regardless of where in a page or in another widget's content slot the File Uploader is placed.

#### Scenario: File action button inside a form

- **WHEN** the File Uploader is rendered inside a `<form>` and the user clicks a file's action button
- **THEN** the button's configured action is executed
- **AND** the enclosing form is not submitted

#### Scenario: Retry button inside a form

- **WHEN** an upload has failed, the File Uploader is rendered inside a `<form>`, and the user clicks the retry button
- **THEN** the upload is retried
- **AND** the enclosing form is not submitted

#### Scenario: Repeated activation inside a form

- **WHEN** the user clicks a file's action button several times in succession while inside a `<form>`
- **THEN** the enclosing form is not submitted at any point

### Requirement: Action buttons rely on the native button role

File Uploader buttons SHALL be native `<button>` elements and SHALL NOT declare a redundant `role="button"` attribute. The exposed accessibility role SHALL remain `button`.

#### Scenario: Action button role

- **WHEN** a file action button is rendered
- **THEN** it is a native `<button>` element with no explicit `role` attribute
- **AND** it is discoverable by its `button` role
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
## 1. Type the buttons

- [x] 1.1 `src/components/ActionButton.tsx` — add `type="button"` to the `<button>` element
- [x] 1.2 `src/components/ActionButton.tsx` — remove the redundant `role={"button"}` attribute
- [x] 1.3 `src/components/RetryButton.tsx` — add `type="button"` to the `<button>` element
- [x] 1.4 Confirm no other `<button>` elements exist in `src/` (`grep -rn "<button" src`)

## 2. Unit tests

- [x] 2.1 Add `src/components/__tests__/ActionButton.spec.tsx` following the pattern in `src/components/__tests__/DismissActionsBar.spec.tsx`
- [x] 2.2 Test: rendering `ActionButton` inside a `<form onSubmit={spy}>` and clicking it calls the button's `action` and does not call the submit spy
- [x] 2.3 Test: the rendered element is a `<button>` with `type="button"` and no explicit `role` attribute
- [x] 2.4 Add equivalent coverage for `RetryButton` inside a `<form>` (retry runs, form not submitted) — placed in its own `src/components/__tests__/RetryButton.spec.tsx`
- [x] 2.5 Verify the new submit test is meaningful: with `type="button"` temporarily removed, "runs its action without submitting an enclosing form" fails with `Received number of calls: 1`

## 3. Manual verification in Studio Pro

- [ ] 3.1 Place a File Uploader on a page inside a container that renders a `<form>`, upload a file, click its action button — the page does not submit or reload
- [ ] 3.2 Force an upload failure, click retry — retry runs, no form submission
- [ ] 3.3 Keyboard: Tab to a file action button, press Enter and Space — action runs, no form submission
- [ ] 3.4 Confirm clicking an action button still does not activate the surrounding `.file-entry` / dropzone (the `stopPropagation()` behaviour is unchanged)

## 4. Documentation

- [x] 4.1 Add a `CHANGELOG.md` entry under Unreleased/Fixed: file action and retry buttons no longer submit an enclosing form
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
schema: spec-driven

# Project context (optional)
# This is shown to AI when creating artifacts.
# Add your tech stack, conventions, style guides, domain knowledge, etc.
# Example:
# context: |
# Tech stack: TypeScript, React, Node.js
# We use conventional commits
# Domain: e-commerce platform

# Per-artifact rules (optional)
# Add custom rules for specific artifacts.
# Example:
# rules:
# proposal:
# - Keep proposals under 500 words
# - Always include a "Non-goals" section
# tasks:
# - Break tasks into chunks of max 2 hours
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# file-uploader-action-buttons Specification

## Purpose

TBD - created by archiving change fix-untyped-action-buttons. Update Purpose after archive.

## Requirements

### Requirement: Widget buttons do not submit enclosing forms

Every `<button>` element rendered by the File Uploader widget SHALL declare `type="button"`. Activating a file action button or the retry button SHALL run only that button's own action and SHALL NOT cause submission of any enclosing `<form>`, regardless of where in a page or in another widget's content slot the File Uploader is placed.

#### Scenario: File action button inside a form

- **WHEN** the File Uploader is rendered inside a `<form>` and the user clicks a file's action button
- **THEN** the button's configured action is executed
- **AND** the enclosing form is not submitted

#### Scenario: Retry button inside a form

- **WHEN** an upload has failed, the File Uploader is rendered inside a `<form>`, and the user clicks the retry button
- **THEN** the upload is retried
- **AND** the enclosing form is not submitted

#### Scenario: Repeated activation inside a form

- **WHEN** the user clicks a file's action button several times in succession while inside a `<form>`
- **THEN** the enclosing form is not submitted at any point

### Requirement: Action buttons rely on the native button role

File Uploader buttons SHALL be native `<button>` elements and SHALL NOT declare a redundant `role="button"` attribute. The exposed accessibility role SHALL remain `button`.

#### Scenario: Action button role

- **WHEN** a file action button is rendered
- **THEN** it is a native `<button>` element with no explicit `role` attribute
- **AND** it is discoverable by its `button` role
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export function ActionButton({ action, icon, title, isDisabled }: ActionButtonPr
);
return (
<button
role={"button"}
type="button"
className={classNames("action-button", {
disabled: isDisabled
})}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export const RetryButton = observer(function RetryButton({ store }: RetryButtonP

return (
<button
type="button"
className="retry-button"
disabled={!store.canRetry}
onClick={onClick}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import "@testing-library/jest-dom";
import { fireEvent, render, screen } from "@testing-library/react";
import { FormEvent } from "react";
import { ActionButton } from "../ActionButton";

jest.mock("../../utils/mx-data", () => ({
fetchDocumentUrl: jest.fn(),
fetchImageThumbnail: jest.fn(),
fetchMxObject: jest.fn(),
removeObject: jest.fn(),
saveFile: jest.fn(),
fileHasContents: jest.fn()
}));

function renderInForm(action: jest.Mock): jest.Mock {
const onSubmit = jest.fn((e: FormEvent) => e.preventDefault());

render(
<form onSubmit={onSubmit}>
<ActionButton icon={<span />} title="add" action={action} isDisabled={false} />
</form>
);

return onSubmit;
}

describe("ActionButton", () => {
it("is a native button with an explicit type and no redundant role", () => {
render(<ActionButton icon={<span />} title="add" action={jest.fn()} isDisabled={false} />);

const button = screen.getByRole("button", { name: "add" });
expect(button.tagName).toBe("BUTTON");
expect(button).toHaveAttribute("type", "button");
expect(button).not.toHaveAttribute("role");
});

it("runs its action without submitting an enclosing form", () => {
const action = jest.fn();
const onSubmit = renderInForm(action);

fireEvent.click(screen.getByRole("button", { name: "add" }));

expect(action).toHaveBeenCalledTimes(1);
expect(onSubmit).not.toHaveBeenCalled();
});

it("does not submit an enclosing form on repeated activation", () => {
const action = jest.fn();
const onSubmit = renderInForm(action);

const button = screen.getByRole("button", { name: "add" });
fireEvent.click(button);
fireEvent.click(button);
fireEvent.click(button);

expect(action).toHaveBeenCalledTimes(3);
expect(onSubmit).not.toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import "@testing-library/jest-dom";
import { fireEvent, render, screen } from "@testing-library/react";
import { FormEvent } from "react";
import { FileUploaderContainerProps } from "../../../typings/FileUploaderProps";
import { FileStore } from "../../stores/FileStore";
import { TranslationsStoreProvider } from "../../utils/useTranslationsStore";
import { RetryButton } from "../RetryButton";

jest.mock("../../utils/mx-data", () => ({
fetchDocumentUrl: jest.fn(),
fetchImageThumbnail: jest.fn(),
fetchMxObject: jest.fn(),
removeObject: jest.fn(),
saveFile: jest.fn(),
fileHasContents: jest.fn()
}));

function makeFakeProps(): FileUploaderContainerProps {
return {
name: "fileUploader1",
retryButtonTextMessage: { value: "Retry upload", status: "available" }
} as unknown as FileUploaderContainerProps;
}

function renderInForm(): { onSubmit: jest.Mock; retry: jest.Mock } {
const retry = jest.fn();
const store = { canRetry: true, retry } as unknown as FileStore;
const onSubmit = jest.fn((e: FormEvent) => e.preventDefault());

render(
<TranslationsStoreProvider props={makeFakeProps()}>
<form onSubmit={onSubmit}>
<RetryButton store={store} />
</form>
</TranslationsStoreProvider>
);

return { onSubmit, retry };
}

describe("RetryButton", () => {
it("declares an explicit button type", () => {
renderInForm();

expect(screen.getByRole("button", { name: "Retry upload" })).toHaveAttribute("type", "button");
});

it("retries the upload without submitting an enclosing form", () => {
const { onSubmit, retry } = renderInForm();

fireEvent.click(screen.getByRole("button", { name: "Retry upload" }));

expect(retry).toHaveBeenCalledTimes(1);
expect(onSubmit).not.toHaveBeenCalled();
});
});
Loading
Loading