Skip to content

macOS: don't crash the process when bridge injection fails on a non-s… - #82

Open
szemeredipeter-prog wants to merge 1 commit into
AvaloniaUI:mainfrom
szemeredipeter-prog:fix/macos-nonhtml-navigation-crash
Open

szemeredipeter-prog wants to merge 1 commit into
AvaloniaUI:mainfrom
szemeredipeter-prog:fix/macos-nonhtml-navigation-crash

Conversation

@szemeredipeter-prog

Copy link
Copy Markdown

…criptable navigation

OnDelegateOnDidFinishNavigation unconditionally injects the C#<->JS bridge script via InvokeScript on every finished navigation. When the navigation loaded non-HTML content (e.g. a file/PDF/image download response), WKWebView.evaluateJavaScript fails at the native WebKit level; InvokeScript already converts that into a JavaScriptException, but nothing caught it in this async void handler, so the exception escaped and crashed the whole process instead of just failing that one bridge-injection attempt.

Reproduced consistently on macOS (Apple Silicon) with a ChatGPT-generated file download: clicking the download link crashed the process every time, with the app otherwise working correctly (login, session persistence, drag-and-drop upload, and clipboard paste all unaffected).

Fix: wrap the InvokeScript call in a try/catch for JavaScriptException. This only stops the crash - it does not add file-download handling (saving, progress, etc.), which is a separate, larger change (decidePolicyForNavigationResponse

  • WKDownloadDelegate) intentionally left out of this PR; happy to open that as its own follow-up if there's interest.

Verified by building this change locally (net8.0 and net10.0 desktop TFMs) and by running AtprismPoc, an Avalonia app that embeds NativeWebView, against a ProjectReference to this patched clone: the same download that reliably crashed the process before no longer does.

…criptable navigation

OnDelegateOnDidFinishNavigation unconditionally injects the C#<->JS bridge
script via InvokeScript on every finished navigation. When the navigation
loaded non-HTML content (e.g. a file/PDF/image download response),
WKWebView.evaluateJavaScript fails at the native WebKit level; InvokeScript
already converts that into a JavaScriptException, but nothing caught it in
this async void handler, so the exception escaped and crashed the whole
process instead of just failing that one bridge-injection attempt.

Reproduced consistently on macOS (Apple Silicon) with a ChatGPT-generated
file download: clicking the download link crashed the process every time,
with the app otherwise working correctly (login, session persistence,
drag-and-drop upload, and clipboard paste all unaffected).

Fix: wrap the InvokeScript call in a try/catch for JavaScriptException. This
only stops the crash - it does not add file-download handling (saving,
progress, etc.), which is a separate, larger change (decidePolicyForNavigationResponse
+ WKDownloadDelegate) intentionally left out of this PR; happy to open that
as its own follow-up if there's interest.

Verified by building this change locally (net8.0 and net10.0 desktop TFMs)
and by running AtprismPoc, an Avalonia app that embeds NativeWebView, against
a ProjectReference to this patched clone: the same download that reliably
crashed the process before no longer does.
Comment on lines +304 to +305
// The just-finished navigation didn't load a scriptable HTML document (e.g. a file
// download, image, or PDF response) - EvaluateJavaScriptAsync then fails at the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should try to detect common document types on which script cannot be executed.

And log error/warning when this "script invoke" failed. There might be situations, when this script call is expected, but exception would be silently ignored after this PR.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also please write a more concise code comment, with more natural line breaks.
The longer comment the less people will actually read it in the future.

This branch has not been deployed

No deployments
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.

2 participants