Skip to content

Fix: handle sms: and tel: links and support regular navigations#5

Open
Coleton (Locrian24) wants to merge 2 commits into
mainfrom
fix/handle-protocols
Open

Fix: handle sms: and tel: links and support regular navigations#5
Coleton (Locrian24) wants to merge 2 commits into
mainfrom
fix/handle-protocols

Conversation

@Locrian24
Copy link
Copy Markdown
Contributor

Description of the change

The WebView previously only handled mailto: (and only for target="_blank" navigations via WKUIDelegate). This adds WKNavigationDelegate with decidePolicyFor to intercept mailto:, sms:, and tel: links on all navigations, handing them off to the OS. The existing createWebViewWith handler now only covers http/https new-window navigations to avoid double-opening external scheme URLs.

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation or Development tools (readme, specs, tests, code formatting)

Links

  • Jira issue number: (PUT IT HERE)
  • Process.st launch checklist: (PUT IT HERE)

Checklists

Development

  • Prettier was run (if applicable)
  • The behaviour changes in the pull request are covered by specs
  • All tests related to the changed code pass in development

Paperwork

  • This pull request has a descriptive title and information useful to a reviewer
  • This pull request has a Jira number
  • This pull request has a Process.st launch checklist

Code review

  • Changes have been reviewed by at least one other engineer
  • Security impacts of this change have been considered

…egular navigations

The WebView previously only handled mailto: (and only for target="_blank"
navigations via WKUIDelegate). This adds WKNavigationDelegate with
decidePolicyFor to intercept mailto:, sms:, and tel: links on all
navigations, handing them off to the OS. The existing createWebViewWith
handler now only covers http/https new-window navigations to avoid
double-opening external scheme URLs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 2, 2026 22:24
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates SaaSquatchWebView’s navigation handling so mailto:, sms:, and tel: links are intercepted on all navigations and opened via the OS, while keeping target="_blank" http/https navigations opening externally.

Changes:

  • Add WKNavigationDelegate and a decidePolicyFor implementation to intercept mailto:, sms:, and tel: schemes.
  • Restrict createWebViewWith handling to only http/https new-window navigations to avoid double-opening external scheme URLs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +140 to +143
if let url = navigationAction.request.url,
let scheme = url.scheme,
Self.externalSchemes.contains(scheme) {
UIApplication.shared.open(url)
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

URL.scheme is case-insensitive by spec, but the externalSchemes lookup is currently case-sensitive. This can fail to intercept links like TEL:/MailTo: coming from HTML. Consider normalizing the scheme (e.g., compare scheme.lowercased() against the set) before checking membership.

Copilot uses AI. Check for mistakes.
…="_blank" links

WKWebView skips decidePolicyFor entirely for target="_blank" navigations,
routing them directly to createWebViewWith. External schemes (mailto, sms,
tel) must be handled in both delegates to cover both navigation paths.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@curtinmartis
Copy link
Copy Markdown

Coleton (@Locrian24) We are running into this exact issue while integrating the Impact SDK into our iOS app. Do you have any idea what you'd expect the turnaround time to be on getting this fix merged?

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants