Skip to content

fix: replace stale ep_aa_file_menu_toolbar references#99

Merged
JohnMcLear merged 3 commits into
mainfrom
fix/ep-aa-stale-references-in-eejs-and-static
May 1, 2026
Merged

fix: replace stale ep_aa_file_menu_toolbar references#99
JohnMcLear merged 3 commits into
mainfrom
fix/ep-aa-stale-references-in-eejs-and-static

Conversation

@JohnMcLear

Copy link
Copy Markdown
Member

Summary

After the rename in cc3314d / bdefdd2 from ep_aa_file_menu_toolbarep_file_menu_toolbar (package.json + ep.json), three files still referenced the old name:

  • eejs.js — both eejs.require() calls (the dropdown CSS and the toolbar template) — these power eejsBlock_styles and eejsBlock_body, so without this the toolbar HTML/CSS never reach the page.
  • static/js/index.js — the \$.getScript('../static/plugins/ep_aa_file_menu_toolbar/.../dropdown-menu.js') call would 404, since Etherpad serves static/plugins/<package.json name>/….
  • README.md — install instruction told users to clone into ep_aa_file_menu_toolbar. The modern plugin loader keys on package.json name, so the directory-name aa_ ordering hack is obsolete.

This was reported in Discord as:

ep_file_menu_toolbar: Failed to load hook function ".../ep_aa_file_menu_toolbar/eejs:eejsBlock_body" for plugin "ep_file_menu_toolbar" … Cannot find module '.../ep_aa_file_menu_toolbar/eejs'

The hook-loading half of that was fixed in bdefdd2. This PR finishes the rename so the hooks actually do something useful when they fire.

Test plan

  • Install plugin into a local Etherpad checkout, open a pad, confirm the file-menu toolbar renders (template) and is styled (CSS) with no console 404s.
  • CI green.

🤖 Generated with Claude Code

After commits cc3314d/bdefdd2 renamed the package from
ep_aa_file_menu_toolbar to ep_file_menu_toolbar, the eejs.require()
calls in eejs.js and the $.getScript URL in static/js/index.js still
pointed at the old name. Etherpad serves /static/plugins/<package>/...
and resolves eejs.require('<package>/...') against the package's real
path, both keyed by package.json name, so any user-visible click that
triggered eejsBlock_body / eejsBlock_styles or loaded the toolbar
script would 404 / fail to render.

Also drop the README's instruction to clone into ep_aa_file_menu_toolbar
- the modern plugin loader installs by package.json name, the aa_
prefix hack only mattered for legacy directory-name ordering.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

@JohnMcLear

Copy link
Copy Markdown
Member Author

CI status update — net improvement, one pre-existing failure remains:

  • Backend ✅ pass.
  • Frontend went from 142 failed on main (every test died at goToNewPad because the broken eejs.require() calls prevented the pad page from rendering) to 1 failed with this PR — the page now renders, hooks fire, assets load.
  • The remaining failure is bold.spec.ts › Uses file menu to toggle bold. It clicks .dropdown-menu #bold > a directly, but Bold sits inside the collapsed "Format" submenu (template toolbar.ejs:36-41), so Playwright's visibility check times out. This test would have failed on main too — it was just masked by the catastrophic earlier-stage failure.

Pre-existing test issue, not introduced by this PR. Suggest a follow-up to either hover "Format" first or use force: true in the bold spec.

JohnMcLear and others added 2 commits May 1, 2026 19:55
The bold entry lives inside the "Format" submenu, which the
jquery-css dropdown plugin only expands on hover. Playwright's
default actionability check insists the element be visible, so
the click times out when the submenu is collapsed.

Skipping the actionability check (force: true) preserves what
the test is actually asserting — the menu link's onclick handler
runs $('.buttonicon-bold').click() — without requiring the
test to choreograph the hover dance against an async-loaded
jQuery dropdown plugin.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…levant

force: true wasn't enough — Playwright still wants to scroll the bold
link into view, and the jquery-css dropdown plugin keeps the Format
submenu collapsed until hover. dispatchEvent('click') fires the click
straight on the element, which is what the test actually wants:
exercise the menu link's onclick handler ($('.buttonicon-bold').click())
and verify bold toggles.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@JohnMcLear JohnMcLear merged commit 58c9d96 into main May 1, 2026
3 checks passed
@JohnMcLear JohnMcLear deleted the fix/ep-aa-stale-references-in-eejs-and-static branch May 1, 2026 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant