fix(v3/windows): Fix ensure JS string is executed when webview is created#4876
Closed
popaprozac wants to merge 7 commits intowailsapp:v3-alphafrom
Closed
fix(v3/windows): Fix ensure JS string is executed when webview is created#4876popaprozac wants to merge 7 commits intowailsapp:v3-alphafrom
popaprozac wants to merge 7 commits intowailsapp:v3-alphafrom
Conversation
Contributor
WalkthroughGates execution of user-provided JS and CSS to the first Windows WebView navigation by removing their one-time setup in Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @v3/pkg/application/webview_window_windows.go:
- Around line 2119-2128: The code references an undefined variable cssEscaped
when embedding w.parent.options.CSS into a JS string inside the webview
navigation block; define cssEscaped by taking w.parent.options.CSS and escaping
single quotes, backslashes and newlines (e.g., replace \ with \\, ' with \', and
normalize newlines to \n) before using fmt.Sprintf, so update the block in
webview_window_windows.go where execJS is called (look for
webviewNavigationCompleted and execJS) to compute cssEscaped from
w.parent.options.CSS and use that escaped value in the generated JS string.
🧹 Nitpick comments (1)
v3/UNRELEASED_CHANGELOG.md (1)
26-26: Minor grammar issue in changelog entry.The phrasing "Fix adds missing" is awkward. Consider rewording to follow the pattern used in the example entries.
📝 Suggested fix
-- Fix adds missing execJS call when creating Windows webview +- Fix missing execJS call when creating Windows webview
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
v3/UNRELEASED_CHANGELOG.mdv3/pkg/application/webview_window_windows.go
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-13T19:52:13.812Z
Learnt from: leaanthony
Repo: wailsapp/wails PR: 4783
File: v3/pkg/events/events.go:72-100
Timestamp: 2025-12-13T19:52:13.812Z
Learning: In Wails v3, the linux:WindowLoadChanged event was intentionally removed as a breaking change and replaced with four granular WebKit2 load events: linux:WindowLoadStarted, linux:WindowLoadRedirected, linux:WindowLoadCommitted, and linux:WindowLoadFinished. Users should migrate to linux:WindowLoadFinished for detecting when the WebView has finished loading.
Applied to files:
v3/pkg/application/webview_window_windows.go
📚 Learning: 2025-10-17T23:16:11.570Z
Learnt from: Sammy-T
Repo: wailsapp/wails PR: 4570
File: v2/internal/frontend/desktop/linux/window_webkit6.go:97-108
Timestamp: 2025-10-17T23:16:11.570Z
Learning: For webkit_6/GTK4 builds in v2/internal/frontend/desktop/linux/window_webkit6.go, GTK widget creation should not be wrapped in invokeOnMainThread. The activation mechanism (activateWg + onActivate export) already handles thread safety, and additional wrapping would cause issues.
Applied to files:
v3/pkg/application/webview_window_windows.go
📚 Learning: 2025-12-29T08:02:15.134Z
Learnt from: popaprozac
Repo: wailsapp/wails PR: 4839
File: docs/src/content/docs/reference/window.mdx:616-620
Timestamp: 2025-12-29T08:02:15.134Z
Learning: In Wails v3, the correct API for creating windows is `app.Window.New()` and `app.Window.NewWithOptions(...)`, not `app.NewWebviewWindow()` or `app.NewWebviewWindowWithOptions(...)`. The Application struct exposes a Window field of type *WindowManager that provides these methods.
Applied to files:
v3/pkg/application/webview_window_windows.go
📚 Learning: 2026-01-04T08:01:00.038Z
Learnt from: symball
Repo: wailsapp/wails PR: 4853
File: v2/internal/system/system.go:128-152
Timestamp: 2026-01-04T08:01:00.038Z
Learning: In v2/internal/system/system.go, shared functions like checkLibrary are defined without build tags but are only invoked from platform-specific files (system_linux.go, system_windows.go, system_darwin.go) that have build constraints. Reviewers should ensure there are no runtime OS checks in system.go and that platform-specific behavior is controlled via build tags. If runtime switches exist, remove them in favor of compile-time platform constraints to reduce overhead and improve correctness.
Applied to files:
v3/pkg/application/webview_window_windows.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
- GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
- GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
- GitHub Check: semgrep-cloud-platform/scan
popaprozac
commented
Jan 12, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @v3/pkg/application/webview_window_windows.go:
- Around line 2119-2129: The injected JS/CSS is being applied twice when
w.parent.options.HTML != "" because chromium.Init(script) in setupChromium()
already injects them, and the navigation handler later re-injects while
webviewNavigationCompleted is still false; to fix, either set
w.webviewNavigationCompleted = true in the HTML branch inside setupChromium()
(the code around setupChromium() / chromium.Init and the HTML path) to avoid the
later injection, or remove the initial injection and consolidate into the
navigation-completion block (the block using w.execJS), and also fix CSS
escaping by replacing the ad-hoc single-quote replace with a robust escape
(e.g., use fmt.Sprintf with %q or a JSON/string encoder) when building the style
script before calling w.execJS.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
v3/pkg/application/webview_window_windows.go
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: APshenkin
Repo: wailsapp/wails PR: 4480
File: v2/internal/frontend/desktop/darwin/message.h:17-19
Timestamp: 2025-08-08T09:13:16.916Z
Learning: In Wails v2 bindings origin verification, processBindingMessage intentionally has different signatures across platforms: Darwin includes an isMainFrame bool (WKWebKit provides it), Linux uses two params (message, source) as WebKitGTK doesn’t expose main-frame info there, and Windows handles origin checks in Go via WebView2 sender/args without a C bridge. This divergence is acceptable/expected per maintainer (APshenkin).
Learnt from: leaanthony
Repo: wailsapp/wails PR: 4783
File: v3/pkg/events/events.go:72-100
Timestamp: 2025-12-13T19:52:13.812Z
Learning: In Wails v3, the linux:WindowLoadChanged event was intentionally removed as a breaking change and replaced with four granular WebKit2 load events: linux:WindowLoadStarted, linux:WindowLoadRedirected, linux:WindowLoadCommitted, and linux:WindowLoadFinished. Users should migrate to linux:WindowLoadFinished for detecting when the WebView has finished loading.
📚 Learning: 2025-12-13T19:52:13.812Z
Learnt from: leaanthony
Repo: wailsapp/wails PR: 4783
File: v3/pkg/events/events.go:72-100
Timestamp: 2025-12-13T19:52:13.812Z
Learning: In Wails v3, the linux:WindowLoadChanged event was intentionally removed as a breaking change and replaced with four granular WebKit2 load events: linux:WindowLoadStarted, linux:WindowLoadRedirected, linux:WindowLoadCommitted, and linux:WindowLoadFinished. Users should migrate to linux:WindowLoadFinished for detecting when the WebView has finished loading.
Applied to files:
v3/pkg/application/webview_window_windows.go
📚 Learning: 2025-10-17T23:16:11.570Z
Learnt from: Sammy-T
Repo: wailsapp/wails PR: 4570
File: v2/internal/frontend/desktop/linux/window_webkit6.go:97-108
Timestamp: 2025-10-17T23:16:11.570Z
Learning: For webkit_6/GTK4 builds in v2/internal/frontend/desktop/linux/window_webkit6.go, GTK widget creation should not be wrapped in invokeOnMainThread. The activation mechanism (activateWg + onActivate export) already handles thread safety, and additional wrapping would cause issues.
Applied to files:
v3/pkg/application/webview_window_windows.go
📚 Learning: 2025-02-24T06:08:55.645Z
Learnt from: popaprozac
Repo: wailsapp/wails PR: 4098
File: v3/pkg/services/notifications/notifications_windows.go:91-113
Timestamp: 2025-02-24T06:08:55.645Z
Learning: The JSON quote handling in Windows notifications' activation arguments (v3/pkg/services/notifications/notifications_windows.go) has a known limitation with single-quote collisions that needs to be addressed after initial testing.
Applied to files:
v3/pkg/application/webview_window_windows.go
📚 Learning: 2025-08-08T13:15:37.785Z
Learnt from: APshenkin
Repo: wailsapp/wails PR: 4484
File: v2/internal/frontend/utils/urlValidator_test.go:152-155
Timestamp: 2025-08-08T13:15:37.785Z
Learning: In v2/internal/frontend/utils/urlValidator_test.go, the project accepts a literal '|' in the URL path (e.g., "https://example.com/user/123|admin") as valid for BrowserOpenURL. Maintainer (APshenkin) clarified that browsers will escape it automatically; thus, tests should not require percent-encoding '%7C' nor expect an error from utils.ValidateURL for this case. Avoid suggesting rejection or forced encoding of '|' in path in future reviews.
Applied to files:
v3/pkg/application/webview_window_windows.go
📚 Learning: 2026-01-04T08:01:00.038Z
Learnt from: symball
Repo: wailsapp/wails PR: 4853
File: v2/internal/system/system.go:128-152
Timestamp: 2026-01-04T08:01:00.038Z
Learning: In v2/internal/system/system.go, shared functions like checkLibrary are defined without build tags but are only invoked from platform-specific files (system_linux.go, system_windows.go, system_darwin.go) that have build constraints. Reviewers should ensure there are no runtime OS checks in system.go and that platform-specific behavior is controlled via build tags. If runtime switches exist, remove them in favor of compile-time platform constraints to reduce overhead and improve correctness.
Applied to files:
v3/pkg/application/webview_window_windows.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
- GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
popaprozac
commented
Jan 12, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @v3/pkg/application/webview_window_windows.go:
- Around line 2111-2114: The JS injection escapes only single quotes and fails
for newlines/backslashes; replace the manual strings.ReplaceAll escaping of
w.parent.options.CSS with proper Go string quoting (use fmt.Sprintf with %q)
when building the js variable so the CSS is safely encoded as a JS string;
update the block that constructs js and calls w.execJS to use
fmt.Sprintf("(function(){...})()", cssQuoted) where cssQuoted is produced via %q
(consistent with setHTML usage) so multiline/backslash characters are handled
correctly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
v3/pkg/application/webview_window_windows.go
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: leaanthony
Repo: wailsapp/wails PR: 4783
File: v3/pkg/events/events.go:72-100
Timestamp: 2025-12-13T19:52:13.812Z
Learning: In Wails v3, the linux:WindowLoadChanged event was intentionally removed as a breaking change and replaced with four granular WebKit2 load events: linux:WindowLoadStarted, linux:WindowLoadRedirected, linux:WindowLoadCommitted, and linux:WindowLoadFinished. Users should migrate to linux:WindowLoadFinished for detecting when the WebView has finished loading.
📚 Learning: 2025-10-17T23:16:11.570Z
Learnt from: Sammy-T
Repo: wailsapp/wails PR: 4570
File: v2/internal/frontend/desktop/linux/window_webkit6.go:97-108
Timestamp: 2025-10-17T23:16:11.570Z
Learning: For webkit_6/GTK4 builds in v2/internal/frontend/desktop/linux/window_webkit6.go, GTK widget creation should not be wrapped in invokeOnMainThread. The activation mechanism (activateWg + onActivate export) already handles thread safety, and additional wrapping would cause issues.
Applied to files:
v3/pkg/application/webview_window_windows.go
📚 Learning: 2025-12-13T19:52:13.812Z
Learnt from: leaanthony
Repo: wailsapp/wails PR: 4783
File: v3/pkg/events/events.go:72-100
Timestamp: 2025-12-13T19:52:13.812Z
Learning: In Wails v3, the linux:WindowLoadChanged event was intentionally removed as a breaking change and replaced with four granular WebKit2 load events: linux:WindowLoadStarted, linux:WindowLoadRedirected, linux:WindowLoadCommitted, and linux:WindowLoadFinished. Users should migrate to linux:WindowLoadFinished for detecting when the WebView has finished loading.
Applied to files:
v3/pkg/application/webview_window_windows.go
📚 Learning: 2025-08-08T09:13:16.916Z
Learnt from: APshenkin
Repo: wailsapp/wails PR: 4480
File: v2/internal/frontend/desktop/darwin/message.h:17-19
Timestamp: 2025-08-08T09:13:16.916Z
Learning: In Wails v2 bindings origin verification, processBindingMessage intentionally has different signatures across platforms: Darwin includes an isMainFrame bool (WKWebKit provides it), Linux uses two params (message, source) as WebKitGTK doesn’t expose main-frame info there, and Windows handles origin checks in Go via WebView2 sender/args without a C bridge. This divergence is acceptable/expected per maintainer (APshenkin).
Applied to files:
v3/pkg/application/webview_window_windows.go
📚 Learning: 2025-12-29T08:02:15.134Z
Learnt from: popaprozac
Repo: wailsapp/wails PR: 4839
File: docs/src/content/docs/reference/window.mdx:616-620
Timestamp: 2025-12-29T08:02:15.134Z
Learning: In Wails v3, the correct API for creating windows is `app.Window.New()` and `app.Window.NewWithOptions(...)`, not `app.NewWebviewWindow()` or `app.NewWebviewWindowWithOptions(...)`. The Application struct exposes a Window field of type *WindowManager that provides these methods.
Applied to files:
v3/pkg/application/webview_window_windows.go
📚 Learning: 2025-02-24T06:08:55.645Z
Learnt from: popaprozac
Repo: wailsapp/wails PR: 4098
File: v3/pkg/services/notifications/notifications_windows.go:91-113
Timestamp: 2025-02-24T06:08:55.645Z
Learning: The JSON quote handling in Windows notifications' activation arguments (v3/pkg/services/notifications/notifications_windows.go) has a known limitation with single-quote collisions that needs to be addressed after initial testing.
Applied to files:
v3/pkg/application/webview_window_windows.go
📚 Learning: 2024-09-20T23:34:29.841Z
Learnt from: nixpare
Repo: wailsapp/wails PR: 3763
File: v3/examples/keybindings/main.go:16-17
Timestamp: 2024-09-20T23:34:29.841Z
Learning: In the codebase, `application.Options.KeyBindings` uses the `application.Window` type, whereas `application.WebviewWindowOptions.KeyBindings` uses `*application.WebviewWindow`. This is intentional and acceptable.
Applied to files:
v3/pkg/application/webview_window_windows.go
📚 Learning: 2026-01-04T08:01:00.038Z
Learnt from: symball
Repo: wailsapp/wails PR: 4853
File: v2/internal/system/system.go:128-152
Timestamp: 2026-01-04T08:01:00.038Z
Learning: In v2/internal/system/system.go, shared functions like checkLibrary are defined without build tags but are only invoked from platform-specific files (system_linux.go, system_windows.go, system_darwin.go) that have build constraints. Reviewers should ensure there are no runtime OS checks in system.go and that platform-specific behavior is controlled via build tags. If runtime switches exist, remove them in favor of compile-time platform constraints to reduce overhead and improve correctness.
Applied to files:
v3/pkg/application/webview_window_windows.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
- GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
v3/pkg/application/webview_window_windows.go (1)
2106-2116: Logic for first-navigation gate is correct.The guard condition properly ensures user-provided JS and CSS are executed only on the initial navigation, before
webviewNavigationCompletedis set totrueat line 2126. This aligns with the PR objective to fix the missingexecJScall during webview creation.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Member
|
That's odd. Any execJS called before the DOM is ready should be buffered and flushed. How did you notice this fail? |
Contributor
Author
|
Sorry I should have been more clear. When setting a On macOS and Linux, this executes after the initial navigation I went to look at the go-webview2 pkg and now see that |
popaprozac
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@v3/pkg/application/webview_window_windows.go`:
- Around line 2106-2116: The block that injects JS/CSS when
!w.webviewNavigationCompleted (using w.webviewNavigationCompleted,
w.parent.options.JS, w.parent.options.CSS and w.execJS) has an extra closing
brace '}' after the CSS injection that breaks compilation; remove that stray '}'
so the if-blocks and function braces properly balance, leaving the outer
function and the if !w.webviewNavigationCompleted { ... } block correctly closed
only once.
|
6 tasks
Member
|
Hey @popaprozac! Your PR got me thinking... the injection stuff has always been a bit messy. Now we are using http semantics, whether we could just request these as files at startup. That way we don't have gnarly escape issues and it works the same for all platforms. I've got a PR up here: #4888 and would be interested in your thoughts 👍 |
Contributor
Author
|
Okay sounds great! Will review but seems like a clear improvement |
Contributor
Author
|
Closing in favor of #4888 |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Description
Currently the
JSandCSSstrings passed inapplication.WebviewWindowOptionswhen creating a new webview on Windows are only executed if anHTMLstring is also passed.This first draft runs them independently after the initial navigation completes. After review may refactor to use the original
chromium.Init()call.Fixes # (issue)
Type of change
Please select the option that is relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using
wails doctor.If you checked Linux, please specify the distro and version.
Test Configuration
Checklist:
website/src/pages/changelog.mdxwith details of this PRSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.