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
15 changes: 15 additions & 0 deletions src/static/js/pad_editbar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,21 @@ exports.padeditbar = new class {
this._bodyKeyEvent(evt);
});

// After any toolbar-select change (e.g. ep_headings style picker,
// ep_font_size), return keyboard focus to the pad editor so the caret
// is back at its previous location. Plugin-provided <select> elements
// aren't always wired through Button.bind (which requires data-key on
// the wrapping <li>); covering them at the #editbar level means every
// toolbar dropdown restores focus consistently. setTimeout(0) defers
// the focus call until plugin change handlers (bound on the same
// event) have finished, so their ace.callWithAce work is done before
// we return focus. Fixes #7589.
$('#editbar').on('change', 'select', () => {
setTimeout(() => {
if (padeditor.ace) padeditor.ace.focus();
}, 0);
});

$('.show-more-icon-btn').on('click', () => {
$('.toolbar').toggleClass('full-icons');
});
Expand Down
39 changes: 39 additions & 0 deletions src/tests/frontend-new/specs/select_focus_restore.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import {expect, test} from '@playwright/test';
import {getPadBody, goToNewPad} from '../helper/padHelper';

test.beforeEach(async ({page}) => {
await goToNewPad(page);
});

test('toolbar select change returns focus to the pad editor (#7589)', async ({page}) => {
// Regression: after picking a value from a toolbar select (ep_headings
// style picker is the canonical example), the caret should return to
// the pad editor so typing continues instead of being swallowed by
// the select wrapper.
const hs = page.locator('#heading-selection');
if ((await hs.count()) === 0) {
test.skip(true, 'ep_headings2 not enabled in this environment');
return;
}

const padBody = await getPadBody(page);
await padBody.click();
await page.keyboard.type('before');

// Change the heading style. The native <select> is hidden behind the
// nice-select wrapper, which on option click does `val(x).trigger('change')`
// internally (see src/static/js/vendors/nice-select.ts). Replicate that
// directly rather than trying to click through the wrapper UI.
await hs.evaluate((el: HTMLSelectElement) => {
el.value = '0';
el.dispatchEvent(new Event('change', {bubbles: true}));
});

// After the change, keyboard input should go into the pad, not the
// toolbar. Write a marker and verify both chunks appear in the pad.
await page.keyboard.type('after');
await page.waitForTimeout(200);
const bodyText = await padBody.innerText();
Comment on lines +27 to +36
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Flaky async focus test 🐞 Bug ☼ Reliability

The focus restore in pad_editbar.ts is deferred via setTimeout(0), but the Playwright spec types
immediately after dispatching the change event, so early keystrokes can be sent before focus returns
to the editor. This can cause intermittent failures depending on event-loop timing and test runner
load.
Agent Prompt
### Issue description
The test types immediately after triggering the select `change`, but the product code restores focus asynchronously via `setTimeout(0)`. This introduces a race where some keystrokes can be sent before focus is back on the Ace editor.

### Issue Context
The implementation in `pad_editbar.ts` defers the focus call specifically to run after other `change` handlers and after nice-select's own focus behavior.

### Fix Focus Areas
- src/tests/frontend-new/specs/select_focus_restore.spec.ts[23-38]

### Suggested change
After dispatching the `change` event, explicitly wait until the editor has focus before calling `page.keyboard.type('after')`.

Example approaches (pick one):
- `await expect(page.locator('iframe[name="ace_outer"]')).toBeFocused();`
- `await page.waitForFunction(() => (document.activeElement as HTMLElement | null)?.getAttribute?.('name') === 'ace_outer');`

Also replace `waitForTimeout(200)` with an `expect.poll(...)` / `await expect(padBody).toContainText(...)`-style wait to reduce flakiness.

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

expect(bodyText).toContain('before');
expect(bodyText).toContain('after');
});
Loading