Skip to content

Fix WebKit keyboard-focus e2e failure - #1

Open
721806280 wants to merge 2 commits into
mainfrom
fix/webkit-focus
Open

721806280 wants to merge 2 commits into
mainfrom
fix/webkit-focus

Conversation

@721806280

Copy link
Copy Markdown
Owner

Diagnostic branch. e2e temporarily scoped to WebKit projects for fast CI feedback while iterating the Linux-WebKit keyboard-focus fix. The scoping commit is reverted before merge.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7c1e55a352

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/utils/diffReview.ts
Comment on lines 189 to 191
// to the other side rather than leave focus where it was.
if (document.activeElement === element) return;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Restore the unsuppressed WebKit focus retry

When keyboard navigation targets a difference that is still below the fold, Linux WebKit can silently ignore focus({ preventScroll: true }); this loop then exhausts its candidates and leaves focus on <body>. focusDiff scrolls smoothly before this call, so the target can still be off-screen at this point, and the existing keyboard-navigation e2e assertion for [data-diff-id].focus-diff:focus will fail on the WebKit projects now selected by CI. Retry focus() without preventScroll after these attempts fail.

Useful? React with 👍 / 👎.

Comment thread .github/workflows/ci.yml
Comment on lines 90 to +91
- name: Run Playwright tests
run: pnpm test:e2e
run: pnpm test:e2e --project=webkit --project=mobile-webkit

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Restore the full browser matrix in CI

This command runs only the two WebKit projects, while playwright.config.ts also defines the desktop Chrome, mobile Chrome, and Firefox projects. Because deployment requires only this e2e job to succeed, regressions in the primary Chromium and Firefox flows can now merge and deploy without any end-to-end coverage; the installed Chrome and Firefox browsers are unused. Restore the unfiltered test command before merging.

Useful? React with 👍 / 👎.

This branch has not been deployed

No deployments
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