Skip to content

test: skip the floating connected footer stroke test in WebKit - #347158

Merged
Bryan Chen (bryanchen-d) merged 1 commit into
mainfrom
copilot/vscode/webkit-footer-stroke-skip
Nov 9, 2026
Merged

Bryan Chen (bryanchen-d) merged 1 commit into
mainfrom
copilot/vscode/webkit-footer-stroke-skip

Conversation

@bryanchen-d

Copy link
Copy Markdown
Collaborator

ModernUIContribution › floating connected footers paint through the strip to the rounded card perimeter, added in #347040, fails in WebKit on all 12 of its cases (strokeVisible: false, true expected). It failed deterministically in main CI 494297 and 494318. Only the macOS arm64 Unit Tests (Node.js, Browser & Copilot) job runs WebKit, and that job gates the macOS arm64 and universal builds, so the 23:00 UTC insider would lose them.

This skips only that test when isSafari, with a TODO for the author. Chromium still runs it.

Why WebKit differs

To let the connected footer's stroke paint in the floating card's 1 px border, connectedPaneTabs.css gives the card overflow: clip; overflow-clip-margin: border-box. WebKit doesn't support overflow-clip-margin (CSS.supports('overflow-clip-margin', 'border-box') is false there), so it clips at the padding edge and hides the stroke in the border. This affects Safari on the web only. Desktop runs on Electron (Chromium).

A clip-path: inset(0 round <radius>) fallback paints the stroke in both engines, but the card's corner radii vary by layout, so that fix is left to the author. The TODO points at it.

Validation

  • Local Playwright WebKit on main: 110 passing, 1 failing, with the same assertion as CI. With this change: 110 passing, 1 pending.
  • Chromium: 111 passing.
  • eslint clean.

Authored with Copilot

Copilot AI balanced review requested due to automatic review settings November 9, 2026 19:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues remain for the WebKit-specific test skip.

No open issues 🎉

What changed in this PR

Temporarily skips the floating connected-footer stroke test in WebKit to unblock macOS arm64 CI, while preserving Chromium coverage. Production rendering is unchanged.

Changes:

  • Imports isSafari and conditionally skips only the affected test.
  • Adds a TODO explaining WebKit’s missing overflow-clip-margin support and suggesting a fallback before re-enabling coverage.
File Description
src/​vs/​workbench/​contrib/​modernUI/​test/​browser/​modernUI.contribution.test.ts Adds the WebKit-specific test skip and documents the rendering limitation.

Review effort: Balanced

@bryanchen-d

Copy link
Copy Markdown
Collaborator Author

Lee Murray (@mrleemurray) heads up that this is failing CI.

It fails in WebKit since #347040, which blocks the macOS arm64 unit tests that gate the macOS arm64 and universal builds. WebKit doesn't support overflow-clip-margin, so overflow: clip hides the connected footer stroke in the card border.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues were identified for the narrowly scoped WebKit-specific test skip.

No open issues 🎉

Review effort: Balanced

@bryanchen-d
Bryan Chen (bryanchen-d) marked this pull request as ready for review November 9, 2026 22:26
@bryanchen-d
Bryan Chen (bryanchen-d) enabled auto-merge (squash) November 9, 2026 22:26
@bryanchen-d
Bryan Chen (bryanchen-d) merged commit 08b9cb5 into main Nov 9, 2026
53 checks passed
@bryanchen-d
Bryan Chen (bryanchen-d) deleted the copilot/vscode/webkit-footer-stroke-skip branch November 9, 2026 22:28
@vs-code-engineering vs-code-engineering Bot added this to the 1.147.0 milestone Nov 9, 2026
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.

3 participants