Skip to content

Widen NTP main scrollbar and fix Cursor format hook on Windows#2851

Open
anpete wants to merge 5 commits into
mainfrom
bugfix/anpete/ntp-scrollbar-and-hook
Open

Widen NTP main scrollbar and fix Cursor format hook on Windows#2851
anpete wants to merge 5 commits into
mainfrom
bugfix/anpete/ntp-scrollbar-and-hook

Conversation

@anpete

@anpete anpete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

https://app.asana.com/1/137249556945/project/1209363561374085/task/1209458074266070?focus=true

Description

  • Increase the NTP main page vertical scrollbar width from 4px to 10px (WebKit-only styling unchanged otherwise).
  • Replace the Cursor afterFileEdit bash format hook with a Node script so Windows does not spawn visible bash --login terminals on every agent edit.

Testing Steps

  • Open NTP static preview from the Build Branch comment (?platform=windows) and confirm the main scrollbar is slightly wider than before, with no top/bottom arrow buttons.
  • On Windows with Cursor, make an agent file edit and confirm Prettier still runs without blank terminal windows (reload Cursor after pulling if needed).

Checklist

  • I have tested this change locally
  • I have tested this change locally in all supported browsers
  • This change will be visible to users
  • I have added automated tests that cover this change
  • I have ensured the change is gated by config
  • This change was covered by a ship review
  • This change was covered by a tech design
  • Any dependent config has been merged

Note

Low Risk
Dev-only hook change plus a small CSS tweak; no auth, data, or core product logic.

Overview
NTP: The main scroller’s WebKit scrollbar width in App.module.css is increased from 4px to 10px; track and thumb styling are unchanged.

Cursor: The afterFileEdit hook in hooks.json now runs node .cursor/hooks/format.mjs instead of format.sh. The new script reads hook JSON from stdin, resolves the edited path, and runs Prettier via require.resolve with windowsHide and ignored stdio so Windows does not flash bash --login terminals. format.sh is removed. Errors are swallowed so formatting never blocks edits.

Reviewed by Cursor Bugbot for commit 7deaf33. Bugbot is set up for automated code reviews on this repo. Configure here.

anpete added 5 commits July 9, 2026 11:27
Use theme tokens for the main page scroller (12px, 15px on Windows) so it matches the customizer drawer, and run Prettier from a Node afterFileEdit hook to avoid spawning visible bash terminals on Windows.
Stick to WebKit-only styling so Windows does not fall back to classic scrollbars with arrow buttons.
Revert theme tokens and standard scrollbar properties; only widen the WebKit track from 4px to 8px.
@github-actions github-actions Bot added the semver-patch Bug fix / internal — no release needed label Jul 9, 2026
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

[Beta] Generated file diff

Time updated: Thu, 09 Jul 2026 12:56:46 GMT

Apple
    - apple/pages/new-tab/dist/index.css

File has changed

Integration
    - integration/pages/new-tab/dist/index.css

File has changed

Windows
    - windows/pages/new-tab/dist/index.css

File has changed

@anpete
anpete marked this pull request as ready for review July 9, 2026 12:51
@github-actions
github-actions Bot requested a review from sashalavron July 9, 2026 12:52

@cursor cursor Bot 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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 7deaf33. Configure here.

Comment thread .cursor/hooks/format.mjs

if (!existsSync(absolutePath)) {
return;
}

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.

Hook skips files not directories

Low Severity

The new hook treats any existing path with existsSync as formatable. The removed format.sh used test -f and exited when the path was not a regular file, so a directory path would not invoke Prettier on a folder.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7deaf33. Configure here.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

⚠️ Cursor review was not successful.

This PR requires a manual review and approval from a member of one of the following teams:

  • @duckduckgo/content-scope-scripts-owners
  • @duckduckgo/apple-devs
  • @duckduckgo/android-devs
  • @duckduckgo/team-windows-development
  • @duckduckgo/extension-owners
  • @duckduckgo/config-aor
  • @duckduckgo/breakage-aor
  • @duckduckgo/breakage

@cursor cursor Bot 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.

Injected PR Evaluation: Web Compatibility & Security

Scope: 4 files changed — .cursor/hooks/ dev tooling (bash → Node) + NTP scrollbar CSS (4px10px). Zero changes to injected/, messaging/, wrapper utilities, captured globals, or message bridge.


Web Compatibility Assessment

File Lines Severity Finding
(none — injected) No injected/src/ or third-party page-world code touched.
special-pages/pages/new-tab/app/components/App.module.css 87–88 info WebKit-only ::-webkit-scrollbar styling on .mainScroller (pre-existing pattern). Firefox uses scrollbar-width elsewhere; unchanged here. NTP-only embedded page — no arbitrary-site compat risk.
special-pages/pages/new-tab/app/components/App.module.css 87–88 info Main scrollbar widened from 4px10px, closer to drawer --ntp-drawer-scroll-width: 12px. Layout compensation via .tube { padding-left: calc(100vw - 100%) } (line 19–20) measures actual scrollbar gutter dynamically, so the width change should not cause persistent layout shift.
.cursor/hooks/format.mjs info Dev-only Cursor IDE hook; not bundled into browser builds. No web-compat surface.

Security Assessment

File Lines Severity Finding
(none — injected) No messaging transports, bridge trust boundaries, captured-globals, or postMessage usage changed.
.cursor/hooks/format.mjs 51–91 info Runs in local Node during Cursor afterFileEdit; stdin JSON is IDE-controlled. Uses spawnSync(process.execPath, [prettierCli, ...]) — no shell interpolation, good command-injection hygiene. Fail-open on errors (intentional for dev UX).
.cursor/hooks/format.mjs 82 info Absolute paths outside repo root still reach Prettier (same as deleted format.sh). Acceptable for local dev tooling; not a production attack surface.
special-pages/.../App.module.css 88 info Static CSS literal; no page-derived values, no injection vector.

Risk Level

Low Risk — Cosmetic NTP special-page CSS and local Cursor dev-hook refactor; no injected script, API shim, messaging, or security-boundary changes.


Recommendations

  1. Visual QA (info): Spot-check NTP main scroller on macOS/Windows/iOS/Android WebViews — scrollbar should be easier to grab without layout regression.
  2. Consistency (optional): Consider a shared CSS variable (e.g. --ntp-main-scroll-width) alongside --ntp-drawer-scroll-width so main/drawer scrollbar widths stay aligned in future tweaks.
  3. No injected-specific tests required — change set is outside the injected runtime surface.
Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

@sashalavron

Copy link
Copy Markdown
Contributor

@shakyShane hey could you take a look? just looping you in based on the asana activity https://app.asana.com/1/137249556945/project/1209698262999393/task/1209458074266070?focus=true

@sashalavron
sashalavron requested a review from shakyShane July 13, 2026 13:40
@shakyShane

Copy link
Copy Markdown
Contributor

Thanks @sashalavron - I’m going to give this a test locally since I’m concerned about the differences across platforms (we had a bunch of issues in the past).

@anpete as mentioned why not break out the cursor fixes to a separate PR so we can get that moving

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver-patch Bug fix / internal — no release needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants