You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The looping while in DDG app link should result in the new error page being shown
Checklist
Please tick all that apply:
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
User-facing special-page UX and a new native messaging hook; no auth or sensitive data handling changes.
Overview
Adds a generalPageProblem special-error variant for pages that cannot load in-app (e.g. Safari redirect loops), with optional native title, message, and button overrides and default copy via new locale keys.
For this kind, the UI hides Advanced and shows a primary Open in Browser action that sends a new openInBrowser notification (existing visitSite remains for the advanced “accept risk” flow on other errors). Tab title, headings, and styling align generalPageProblem with SSL-style shield treatment.
Schema, types, docs, sample data, and Playwright/screenshot tests cover the new flow and overrides.
Reviewed by Cursor Bugbot for commit 493338d. Bugbot is set up for automated code reviews on this repo. Configure here.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
No findings. The changes are scoped to the special-error special page UI, schema/types, sample data, integration tests, and locale strings. They do not touch injected runtime code, browser API wrappers/shims, prototype patches, page DOM mutation observers, or platform entry points.
Security Assessment
No findings. The new Safari redirect-loop path reuses the existing paramless visitSite notification, so it does not forward page-derived objects or introduce a nativeData leakage path. The added errorData.url field is not rendered, echoed back to native, used for network requests, or passed to postMessage.
Risk Level
Low Risk: special-page presentation and schema additions only, with no changes to injected API surface, captured globals, message transports, origin validation, or bridge security checks.
Recommendations
Run the targeted special-error integration coverage for the new safariRedirectLoop path on the intended Apple project/build artifact before merge.
Optional cleanup: remove the trailing space from the new safariRedirectLoopPageHeading source string if it is not intentional.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
No findings. The latest diff remains scoped to the special-error special-page UI, schema/types, sample data, locale strings, and integration coverage. It does not touch injected runtime code, browser API wrappers/shims, prototype patches, DOM observers, or platform entry points.
Security Assessment
No findings. The new Safari redirect-loop path reuses the existing paramless visitSite notification, so it does not forward page-derived objects or introduce a nativeData leakage path. The added errorData.url field is not rendered, echoed to native, used for network requests, or sent via postMessage.
Risk Level
Low Risk: special-page presentation and message-schema additions only, with no changes to injected API surface, captured globals, message transports, origin validation, or bridge security checks.
Recommendations
No blocking recommendations. The prior trailing-space copy concern is addressed in the latest sync.
Validation: npm run build --workspace=special-pages and npm run test-int --workspace=special-pages -- pages/special-error/integration-tests/special-error.spec.js --grep "safari redirect loop|opens site in safari" --reporter list passed.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
No findings. The current diff is isolated to the special-error special page UI, schema/types, locale copy, and integration tests. It does not touch injected runtime code, browser API wrappers/shims, prototype patches, page-world DOM observers, or platform entry points.
Security Assessment
No findings. The new generalPageProblem path sends a paramless openInBrowser notification, so it does not forward page-derived objects or introduce a nativeData leakage path. Native-provided title / message / button copy is rendered as text through Preact, and the required url field is not rendered, fetched, posted, or echoed back to native.
Risk Level
Medium Risk: this adds a user-facing special-error path and a new native notification contract, but the scope is limited to the special page and does not alter injected API surface, captured globals, message transports, origin validation, or bridge security checks.
Recommendations
No blocking recommendations. Keep the existing visitSite notification for Advanced/accept-risk flows and the new openInBrowser notification only on the generalPageProblem primary action, as the latest head does.
Validation: git diff --check, npm run build --workspace=special-pages, and npm run test-int --workspace=special-pages -- pages/special-error/integration-tests/special-error.spec.js --reporter list passed.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
No findings. The current diff is isolated to the special-error special page UI, schema/types, locale copy, and integration tests. It does not touch injected runtime code, browser API wrappers/shims, prototype patches, page-world DOM observers, or platform entry points. The latest sync only extracts duplicated primary-button variant mapping into getPrimaryActionButtonVariant().
Security Assessment
No findings. The new generalPageProblem path sends a paramless openInBrowser notification, so it does not forward page-derived objects or introduce a nativeData leakage path. Native-provided title / message / button copy is rendered as text through Preact, and the required url field is not rendered, fetched, posted, or echoed back to native.
Risk Level
Medium Risk: this adds a user-facing special-error path and a new native notification contract, but the scope is limited to the special page and does not alter injected API surface, captured globals, message transports, origin validation, or bridge security checks.
Recommendations
No blocking recommendations. Keep the existing visitSite notification for Advanced/accept-risk flows and the new openInBrowser notification only on the generalPageProblem primary action.
Validation: git diff --check and npm run build --workspace=special-pages passed.
Left a request for Cursor to generate screenshot cases as they are great for spotting regressions on copy and styling. First time trying it, not sure it's gonna work.
The reason will be displayed to describe this comment to others. Learn more.
Web Compatibility Assessment
No findings. The current diff is scoped to the special-error special page UI, message schema/types, locale copy, docs, and integration/screenshot coverage. It does not touch injected runtime code, browser API wrappers/shims, prototype patches, page-world DOM observers, platform entry points, or cross-frame behavior.
Latest sync note: the only runtime change since the previous automation review is a JSDoc type relaxation in Warning.jsx; the rest adds generalPageProblem screenshot tests and snapshots.
Security Assessment
No findings. The generalPageProblem path sends a paramless openInBrowser notification, so it does not forward page-derived objects or introduce a nativeData leakage path. Native-provided title / message / button values are rendered as text through Preact, and the required url field is not rendered, fetched, posted, or echoed back to native.
Risk Level
Medium Risk: this adds a user-facing special-error path and a new native notification contract, but it remains limited to the special page and does not alter injected API surface, captured globals, message transports, origin validation, or bridge security checks.
Recommendations
No blocking recommendations. Keep visitSite reserved for the existing Advanced/accept-risk flow and openInBrowser only for the generalPageProblem primary action.
Validation: I reviewed the full PR diff plus the incremental diff since the prior automation review. GitHub checks showed build/unit/injected jobs passing; special-pages integration and snapshot jobs were still pending at review time.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
semver-minorNew feature — triggers minor version bump
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
github-actions
Bot
added
the
semver-minor
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
daxtheduck
dismissed
their stale review
Asana Task/Github Issue: https://app.asana.com/1/137249556945/project/392891325557410/task/1215121749144047?focus=true
Description
New special error page for Safari redirect loop, as a generic general page problem with parameterised title, etc.
Testing Steps
npm run buildthis branchChecklist
Please tick all that apply:
Note
Low Risk
User-facing special-page UX and a new native messaging hook; no auth or sensitive data handling changes.
Overview
Adds a
generalPageProblemspecial-error variant for pages that cannot load in-app (e.g. Safari redirect loops), with optional nativetitle,message, andbuttonoverrides and default copy via new locale keys.For this kind, the UI hides Advanced and shows a primary Open in Browser action that sends a new
openInBrowsernotification (existingvisitSiteremains for the advanced “accept risk” flow on other errors). Tab title, headings, and styling aligngeneralPageProblemwith SSL-style shield treatment.Schema, types, docs, sample data, and Playwright/screenshot tests cover the new flow and overrides.
Reviewed by Cursor Bugbot for commit 493338d. Bugbot is set up for automated code reviews on this repo. Configure here.