Fix frontend test failures across all browsers by JohnMcLear · Pull Request #7416 · ether/etherpad

JohnMcLear · 2026-03-31T21:15:36Z

Summary

Home button (webkit): Use window.location.origin + '/' instead of fragile window.location.href + "/../.." relative URL that WebKit doesn't resolve
Test initialization: Wait for #editorcontainer.initialized in goToNewPad/goToPad/appendQueryParams so toolbar, chat, and cookie handlers are fully set up before tests run
Sticky chat (firefox): Clear cookies in chat test beforeEach to prevent chatAndUsers cookie from prior tests disabling the checkbox
Editbar test: Wait for navigation after clicking home button before asserting URL

Test plan

Verify all three browsers (Chrome, Firefox, Webkit) pass frontend tests
Verify home button works correctly in Safari/WebKit

🤖 Generated with Claude Code

- Fix home button using fragile relative URL (window.location.href + "/../..") that WebKit doesn't resolve correctly. Use window.location.origin instead. - Wait for #editorcontainer.initialized in goToNewPad/goToPad/ appendQueryParams so toolbar, chat, and cookie handlers are fully set up before tests interact with them. - Clear cookies in chat test beforeEach to prevent chatAndUsers cookie from prior tests disabling the sticky chat checkbox. - Wait for navigation to complete in editbar home button test. Fixes ether#7405 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

qodo-free-for-open-source-projects · 2026-03-31T21:16:27Z

Review Summary by Qodo

Fix frontend test failures across all browsers

🐞 Bug fix 🧪 Tests

Walkthroughs

Description

• Fix home button using window.location.origin instead of fragile relative URL
• Wait for #editorcontainer.initialized selector in pad helper functions
• Clear cookies in chat test to prevent state pollution between tests
• Wait for navigation completion in editbar home button test

Diagram

flowchart LR
  A["Home Button Navigation"] -->|Replace fragile relative URL| B["Use window.location.origin"]
  C["Test Initialization"] -->|Wait for editor container| D["#editorcontainer.initialized"]
  E["Chat Test State"] -->|Clear cookies| F["Prevent test pollution"]
  G["Editbar Navigation"] -->|Wait for URL change| H["Assert navigation complete"]

File Changes

1. src/static/js/pad_editbar.ts 🐞 Bug fix +1/-1

Fix home button navigation for WebKit
• Replace fragile relative URL navigation with window.location.origin + '/'
• Fixes WebKit compatibility issue with home button navigation
src/static/js/pad_editbar.ts

2. src/tests/frontend-new/helper/padHelper.ts 🧪 Tests +3/-0

Wait for editor container initialization in pad helpers

• Add await page.waitForSelector('#editorcontainer.initialized') to appendQueryParams function
• Add await page.waitForSelector('#editorcontainer.initialized') to goToNewPad function
• Add await page.waitForSelector('#editorcontainer.initialized') to goToPad function
• Ensures toolbar, chat, and cookie handlers are fully initialized before tests run

src/tests/frontend-new/helper/padHelper.ts

3. src/tests/frontend-new/specs/chat.spec.ts 🧪 Tests +2/-1

Clear cookies in chat test setup

• Add context parameter to beforeEach hook
• Clear cookies before each test with await context.clearCookies()
• Prevents chatAndUsers cookie from prior tests affecting sticky chat checkbox

src/tests/frontend-new/specs/chat.spec.ts

View more (1)

4. src/tests/frontend-new/specs/editbar.spec.ts 🧪 Tests +1/-0

Wait for navigation in home button test

• Add await page.waitForURL() call after clicking home button
• Waits for navigation to complete before asserting URL changes
• Ensures URL assertion happens after navigation finishes

src/tests/frontend-new/specs/editbar.spec.ts

qodo-free-for-open-source-projects · 2026-03-31T21:16:28Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (4) 📎 Requirement gaps (0)

1. padeditbar home indent mismatch 📘 Rule violation ⚙ Maintainability

Description

The modified home command line is not indented with 2 spaces as required, introducing inconsistent
formatting. This violates the repository indentation standard and can reduce
readability/consistency.

Code

src/static/js/pad_editbar.ts[359]
+      window.location.href = window.location.origin + '/'

Evidence
PR Compliance ID 8 requires spaces only with 2-space indentation. The changed line in
pad_editbar.ts is indented beyond 2 spaces.
src/static/js/pad_editbar.ts[359-359]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A newly modified line does not follow the required 2-space indentation.
## Issue Context
Compliance requires spaces-only and 2-space indentation for all code.
## Fix Focus Areas
- src/static/js/pad_editbar.ts[359-359]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. padHelper added waits misindented 📘 Rule violation ⚙ Maintainability

Description

The newly added waitForSelector('#editorcontainer.initialized') lines are not indented with 2
spaces as required. This introduces formatting that violates the indentation standard.

Code

src/tests/frontend-new/helper/padHelper.ts[114]
+    await page.waitForSelector('#editorcontainer.initialized');

Evidence
PR Compliance ID 8 requires 2-space indentation. The newly added `await
page.waitForSelector('#editorcontainer.initialized');` lines are indented beyond 2 spaces.
src/tests/frontend-new/helper/padHelper.ts[114-114]
src/tests/frontend-new/helper/padHelper.ts[122-122]
src/tests/frontend-new/helper/padHelper.ts[129-129]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Newly added lines in `padHelper.ts` do not follow 2-space indentation.
## Issue Context
Compliance requires spaces-only and 2-space indentation across the codebase.
## Fix Focus Areas
- src/tests/frontend-new/helper/padHelper.ts[114-114]
- src/tests/frontend-new/helper/padHelper.ts[122-122]
- src/tests/frontend-new/helper/padHelper.ts[129-129]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. chat.spec beforeEach misindented 📘 Rule violation ⚙ Maintainability

Description

The newly added context.clearCookies() line (and surrounding modified setup) is not indented with
2 spaces as required. This violates the formatting/indentation compliance rule.

Code

src/tests/frontend-new/specs/chat.spec.ts[R17-18]
+test.beforeEach(async ({ page, context })=>{
+    await context.clearCookies();

Evidence
PR Compliance ID 8 requires 2-space indentation. The added setup code in test.beforeEach is
indented beyond 2 spaces.
src/tests/frontend-new/specs/chat.spec.ts[17-18]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New lines in `chat.spec.ts` do not follow 2-space indentation.
## Issue Context
Compliance requires spaces-only and 2-space indentation.
## Fix Focus Areas
- src/tests/frontend-new/specs/chat.spec.ts[17-18]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View more (2)

4. editbar.spec waitForURL misindented 📘 Rule violation ⚙ Maintainability

Description

The newly added page.waitForURL(...) line is not indented with 2 spaces as required. This violates
the code formatting standard for indentation.

Code

src/tests/frontend-new/specs/editbar.spec.ts[15]
+  await page.waitForURL((url) => !url.pathname.includes('/p/'));

Evidence
PR Compliance ID 8 requires 2-space indentation. The added await page.waitForURL(...) line is
indented beyond 2 spaces.
src/tests/frontend-new/specs/editbar.spec.ts[15-15]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A newly added line in `editbar.spec.ts` does not follow 2-space indentation.
## Issue Context
Compliance requires spaces-only and 2-space indentation.
## Fix Focus Areas
- src/tests/frontend-new/specs/editbar.spec.ts[15-15]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. ~~Home drops URL subpath~~ ☑ 🐞 Bug ≡ Correctness

Description

The editbar Home command now navigates to window.location.origin + '/', which strips any
configured base path (for example, Etherpad hosted at /etherpad) and will redirect users to the
domain root instead of the Etherpad home page.

Code

src/static/js/pad_editbar.ts[359]
+      window.location.href = window.location.origin + '/'

Evidence

Etherpad explicitly supports being hosted under a non-root base path: the pad bootstrap computes a
basePath from the current URL and assigns it to pad.baseURL and plugins.baseURL so that client
requests work under a prefix (e.g. /etherpad). The socket.io connector also documents overriding
the path to support hosting Etherpad at a subpath. Redirecting to window.location.origin + '/'
ignores that base path and therefore breaks navigation in such deployments.

src/static/js/pad_editbar.ts[352-360]
src/templates/padBootstrap.js[15-21]
src/static/js/socketio.ts[5-23]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The Home button in `pad_editbar` redirects to `window.location.origin + '/'`, which breaks installations served from a subpath (e.g. `/etherpad`) by sending users to the domain root.
### Issue Context
The client already computes and uses a base path for subpath hosting (see `padBootstrap.js` setting `pad.baseURL`/`plugins.baseURL`).
### Fix Focus Areas
- src/static/js/pad_editbar.ts[352-360]
### Implementation guidance
Update the Home command to navigate to the Etherpad base path, not the origin root. Prefer an absolute, normalized URL to keep WebKit happy while preserving any prefix, e.g.:
- `window.location.href = new URL('..', window.location.href).href;` when on `/.../p/<padId>`
- or `window.location.href = new URL('../..', window.location.href).href;` (if you want to mirror the previous behavior exactly)
Choose the relative target that resolves to the Etherpad homepage for pad URLs, including when Etherpad is hosted under a reverse-proxy prefix.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

ⓘ The new review experience is currently in Beta. Learn more

qodo-free-for-open-source-projects · 2026-03-31T21:19:10Z

    this.registerDropdownCommand('embed');
    this.registerCommand('home', ()=>{
-      window.location.href = window.location.href + "/../.."
+      window.location.href = window.location.origin + '/'


1. padeditbar home indent mismatch 📘 Rule violation ⚙ Maintainability

The modified home command line is not indented with 2 spaces as required, introducing inconsistent formatting. This violates the repository indentation standard and can reduce readability/consistency.

Agent Prompt

## Issue description A newly modified line does not follow the required 2-space indentation. ## Issue Context Compliance requires spaces-only and 2-space indentation for all code. ## Fix Focus Areas - src/static/js/pad_editbar.ts[359-359]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

qodo-free-for-open-source-projects · 2026-03-31T21:19:10Z

    });
    await page.goto(page.url()+"?"+ searchParams.toString());
    await page.waitForSelector('iframe[name="ace_outer"]');
+    await page.waitForSelector('#editorcontainer.initialized');


2. padhelper added waits misindented 📘 Rule violation ⚙ Maintainability

The newly added waitForSelector('#editorcontainer.initialized') lines are not indented with 2 spaces as required. This introduces formatting that violates the indentation standard.

Agent Prompt

## Issue description Newly added lines in `padHelper.ts` do not follow 2-space indentation. ## Issue Context Compliance requires spaces-only and 2-space indentation across the codebase. ## Fix Focus Areas - src/tests/frontend-new/helper/padHelper.ts[114-114] - src/tests/frontend-new/helper/padHelper.ts[122-122] - src/tests/frontend-new/helper/padHelper.ts[129-129]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

qodo-free-for-open-source-projects · 2026-03-31T21:19:10Z

+test.beforeEach(async ({ page, context })=>{
+    await context.clearCookies();


3. chat.spec beforeeach misindented 📘 Rule violation ⚙ Maintainability

The newly added context.clearCookies() line (and surrounding modified setup) is not indented with 2 spaces as required. This violates the formatting/indentation compliance rule.

Agent Prompt

## Issue description New lines in `chat.spec.ts` do not follow 2-space indentation. ## Issue Context Compliance requires spaces-only and 2-space indentation. ## Fix Focus Areas - src/tests/frontend-new/specs/chat.spec.ts[17-18]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

qodo-free-for-open-source-projects · 2026-03-31T21:19:10Z

  expect(attribute).toBe('pad.toolbar.home.title');

  await homeButton.click();
+  await page.waitForURL((url) => !url.pathname.includes('/p/'));


4. editbar.spec waitforurl misindented 📘 Rule violation ⚙ Maintainability

The newly added page.waitForURL(...) line is not indented with 2 spaces as required. This violates the code formatting standard for indentation.

Agent Prompt

## Issue description A newly added line in `editbar.spec.ts` does not follow 2-space indentation. ## Issue Context Compliance requires spaces-only and 2-space indentation. ## Fix Focus Areas - src/tests/frontend-new/specs/editbar.spec.ts[15-15]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Playwright runs locally and doesn't need Sauce Labs secrets, so there's no reason to limit frontend tests to push events only. Also remove stale Sauce Labs references from workflow names/comments. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The stickToScreen() handler manages checkbox state internally with its own toggle logic and a setTimeout. Playwright's check()/uncheck() methods verify state after clicking, but race with the async toggle, causing "Clicking the checkbox did not change its state" errors. Using click() avoids this — the waitForSelector calls already verify the final state. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Remove force:true from sticky chat checkbox clicks — it can bypass jQuery event handlers preventing stickToScreen() from firing. - Wait for chatbox stickyChat class instead of checkbox state, since stickToScreen() manages the checkbox asynchronously via setTimeout. - Reduce workers from 5 to 2 to avoid overloading the single Etherpad server instance, which causes goToNewPad timeouts on CI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Remove all Sauce Labs references (steps, comments, secrets) from frontend test workflows — Playwright replaced Sauce Labs - Remove unused set-output steps and GIT_HASH exports - Remove stale commented-out code from admin tests - Restrict load test to push events only (no need on PRs) - Fix artifact names to not reference undefined matrix.node Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The label element intercepts pointer events on the checkbox (reported by Webkit). On Chrome/Firefox the checkbox is "not stable" due to animations. Clicking the label is how a real user interacts with it and properly triggers the jQuery click handler. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Use URL API to resolve '../..' relative to current URL instead of hardcoding origin + '/'. This preserves any configured base path (e.g. /etherpad) for reverse-proxy installations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

JohnMcLear · 2026-04-01T07:59:14Z

Addressing Qodo review:

Issues 1-4 (indentation): These are false positives — each added line matches the existing indentation style of its surrounding file. The test files use 4-space indent, editbar.spec.ts uses 2-space, and pad_editbar.ts lines are inside a nested method.

Issue 5 (subpath bug): Fixed in commit 19ae682 — now uses new URL('../..', window.location.href).href which preserves any base path for reverse-proxy installations.

qodo-free-for-open-source-projects Bot reviewed Mar 31, 2026

View reviewed changes

JohnMcLear and others added 6 commits March 31, 2026 22:42

JohnMcLear merged commit 1814718 into ether:develop Apr 1, 2026
13 of 27 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix frontend test failures across all browsers#7416

Fix frontend test failures across all browsers#7416
JohnMcLear merged 7 commits into
ether:developfrom
JohnMcLear:fix/frontend-test-failures

JohnMcLear commented Mar 31, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Mar 31, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Mar 31, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot Mar 31, 2026

Uh oh!

qodo-free-for-open-source-projects Bot Mar 31, 2026

Uh oh!

qodo-free-for-open-source-projects Bot Mar 31, 2026

Uh oh!

qodo-free-for-open-source-projects Bot Mar 31, 2026

Uh oh!

Uh oh!

JohnMcLear commented Apr 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		test.beforeEach(async ({ page, context })=>{
		await context.clearCookies();

Uh oh!

Uh oh!

Conversation

JohnMcLear commented Mar 31, 2026

Summary

Test plan

Uh oh!

qodo-free-for-open-source-projects Bot commented Mar 31, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

qodo-free-for-open-source-projects Bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

qodo-free-for-open-source-projects Bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

qodo-free-for-open-source-projects Bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

qodo-free-for-open-source-projects Bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JohnMcLear commented Apr 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

qodo-free-for-open-source-projects Bot commented Mar 31, 2026 •

edited

Loading