-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix for WebView.EvaluateJavaScriptAsync fails to execute JavaScript containing newline characters #29126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix for WebView.EvaluateJavaScriptAsync fails to execute JavaScript containing newline characters #29126
Conversation
|
Hey there @@praveenkumarkarunanithi! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
| public async Task TestEscapeJsStringHandlesNewlines() | ||
| { | ||
| string scriptWithNewlines = "var x = 5;\r\n" + | ||
| "var y = 10;\r" + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does happen when there is javascript code: var myString = "<NEWLINE HERE>"; console.log(myString);? It would be nice to have such a test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MartyIX , Our fix replaces literal newline characters with spaces to ensure JavaScript executes properly across all platforms. JavaScript string escape sequences (like \n) are not affected since they're represented differently in code:
string jsWithEscapeSequence = "var myString = '\\n';";
since standard practice is to use \n escape sequences to represent newlines in strings,most real-world code won't be affected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My point I wanted to make was simply: Your PR can modify original behavior of user's JavaScript code. I believe it should be mentioned in the PR description. If it is acceptable or not, is not up to me. I just want to make sure it's explicitly acknowledged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw: #29139 was merged to the inflight branch yesterday. Would it make sense to test against that branch if your workaround is still needed?
I'm basically very interested what causes that on Windows you observe "silent failure". Do you possibly know where the limitation come from? Or what is buggy?
mattleibow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR should land after #27528
But, as @MartyIX says, newlines may have meaning in the context of JS. For example, they use backtics to represent the multi line strings:
let poll = `Is .NET MAUI cool?
- Yes!
- Yes!
- Yes!
Wow, so it is!
`
console.log(poll);as well as backslashes:
let poll = 'Is .NET MAUI cool? \n\
- Yes! \n\
- Yes! \n\
- Yes! \n\
Wow, so it is! \n\
'
console.log(poll);We need to add tets for these as well.
@mattleibow , I've optimized the fix to handle both scenarios you mentioned: As recommended, I've worked these changes on top of PR #27528 and updated this PR to include those changes. This approach ensures my PR can be smoothly merged after PR #27528 is completed. |
|
FYI: I opened this issue for Windows' WebView: MicrosoftEdge/WebView2Feedback#5288. The thing is that this maui/src/Controls/src/Core/WebView/WebView.cs Line 136 in 5b82faa
should not be necessary for Windows based on WebView2 documentation:
(Emphasis mine) Sadly, this is not how it work in practice. Anyway, we will see what the guys will say -- either there is a bug in the documentation or in the product. |
There was a problem hiding this 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 pull request fixes a critical issue where WebView's EvaluateJavaScriptAsync method failed to execute JavaScript code containing newline characters. The fix consolidates JavaScript string escaping logic into a shared helper class and improves newline handling by normalizing line endings and properly escaping them for cross-platform compatibility.
- Consolidates duplicate JavaScript escaping logic from WebView and HybridWebView into a shared WebViewHelper class
- Enhances JavaScript string processing to handle newline characters, template literals, and backslash continuations
- Adds comprehensive unit tests covering various JavaScript string escaping scenarios
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Core/src/Handlers/WebView/WebViewHelper.cs | New helper class with improved JavaScript string escaping logic that handles newlines, backticks, and complex quote scenarios |
| src/Core/src/Handlers/HybridWebView/HybridWebViewHandler.cs | Removes duplicate EscapeJsString method and uses the shared WebViewHelper implementation |
| src/Controls/src/Core/WebView/WebView.cs | Removes duplicate EscapeJsString method and uses the shared WebViewHelper implementation |
| src/Controls/tests/Core.UnitTests/WebViewHelperTests.cs | Comprehensive unit tests for the new JavaScript string escaping functionality |
| #endif | ||
|
|
||
| // Escape sequence marker | ||
| js = Regex.Replace(js, @"\\n", NewlineMarker); |
Copilot
AI
Sep 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using string.Replace() instead of regex for this literal string replacement, which would be more efficient.
| js = Regex.Replace(js, @"\\n", NewlineMarker); | |
| js = js.Replace("\\n", NewlineMarker); |
| } | ||
|
|
||
| // Remove backslash-newline continuation | ||
| js = Regex.Replace(js, @"\\[ \t]*\n", string.Empty); |
Copilot
AI
Sep 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This regex operation is called on every JavaScript string. Consider caching the compiled regex or using RegexOptions.Compiled for better performance in repeated calls.
9272b5f to
0d465cf
Compare
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 29126Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 29126" |
Root Cause
The
EvaluateJavaScriptAsyncmethod did not account for multi-line JavaScript strings containing newline characters (\r\n, \r, \n). These newline characters caused script execution issues on platforms like Windows (WebView2) and iOS (WKWebView), either silently failing to execute or throwing runtime exceptions due to unprocessed newline characters in the script string.Description of Change
The JavaScript string is now normalized by replacing all newline characters (\r\n, \r, \n) with a space before execution. This ensures consistent behavior and compatibility across platforms.
Issues Fixed
Fixes #11905
Tested the behaviour in the following platforms
Screenshots