Skip to content

Conversation

@zaniluca
Copy link
Owner

No description provided.

@zaniluca zaniluca self-assigned this Jan 16, 2026
@zaniluca zaniluca linked an issue Jan 16, 2026 that may be closed by this pull request
}}
textZoom={125}
onShouldStartLoadWithRequest={(event) => {
if (!/^[data:text, about:blank]/.test(event.url)) {
Copy link

Choose a reason for hiding this comment

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

Bug: The regex /^[data:text, about:blank]/ uses a character class [] instead of alternation |. This incorrectly keeps many external URLs inside the WebView.
Severity: HIGH

Suggested Fix

Replace the incorrect character class-based regex with one that uses alternation to match the full schemes. The regex should be changed to /^(data:|about:blank)$/ to correctly match data: or about:blank URLs.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/screens/NotificationDetail.tsx#L61

Potential issue: The regular expression on line 61 uses a character class `[]` which
matches any single character within the set, rather than the intended string literals.
The logic is designed to open URLs in an external browser unless they match the pattern.
Because of the incorrect regex, any URL starting with characters like 'd', 'a', 't', or
'e' (e.g., `docs.gitlab.com`) will be incorrectly trapped within the WebView instead of
opening externally. This defeats the primary purpose of the feature, which is to open
notification links in an external browser.

Did we get this right? 👍 / 👎 to inform future reviews.

@zaniluca zaniluca merged commit 3dd9059 into master Jan 27, 2026
4 checks passed
@zaniluca zaniluca deleted the 228-use-external-browser-with-notification-content-links branch January 27, 2026 19:29
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.

Use external browser with notification content links

2 participants