Repository navigation
fix(windows): register message bridge at document-start; support injectedJavaScriptBeforeContentLoaded - #3983
Conversation
…ctedJavaScriptBeforeContentLoaded Two related timing bugs in the messaging path: - window.ReactNativeWebView was injected via ExecuteScriptAsync from NavigationCompleted -- after the page had already finished loading. Any page script that reads window.ReactNativeWebView while the page is still loading (a synchronous inline <script> in <head>, for example) never saw it. - WebMessageReceived was unhooked and re-hooked on every NavigationStarting, which is wasteful and unnecessary. This registers the (unchanged) bridge script once via AddScriptToExecuteOnDocumentCreatedAsync when CoreWebView2 becomes available, so it exists before any page script runs on every subsequent navigation, and registers WebMessageReceived once at the same point. Same mechanism now wires up injectedJavaScriptBeforeContentLoaded, which already existed in the codegen'd prop spec (RCTWebView2Props) but was never read on Windows. Trade-off worth flagging: messagingEnabled / injectedJavaScriptBeforeContentLoaded are now captured once at CoreWebView2 init rather than re-checked on every navigation; changing either prop after first mount takes effect on remount rather than immediately. Happy to add proper add/remove-script tracking (RemoveScriptToExecuteOnDocumentCreated) if that dynamic-update behavior matters to you. Validation: this is the same document-start registration pattern (and the same injectedJavaScriptBeforeContentLoaded semantics) running in production in a parallel Windows new-arch WebView2 Fabric component (Facilitron FIT, react-native-windows 0.83.2) -- see FacilitronWorks/react-native-webview-windows. Not compiled against this repo in this change; needs CI here to validate.
…jectedJavaScriptBeforeContentLoaded changes messagingEnabled and injectedJavaScriptBeforeContentLoaded were captured once at CoreWebView2 init, so toggling either after mount only took effect on remount. iOS honors post-mount changes via -resetupScripts (removeAllUserScripts + re-add) and Android re-runs callInjectedJavaScriptBeforeContentLoaded from onPageStarted, so Windows was the odd one out. Tracks the winrt::hstring IDs returned by AddScriptToExecuteOnDocumentCreatedAsync and removes the previous scripts with RemoveScriptToExecuteOnDocumentCreated before re-adding, so a prop change takes effect on the next navigation. UpdateProps captures the old values before any member is overwritten and compares afterwards, so the dirty check is old-vs-new by construction rather than by statement order. Also stops gating injectedJavaScriptBeforeContentLoaded on messagingEnabled, to match iOS (adds atStartScript outside its messagingEnabled branch) and Android. Ownership window closed: once AddScriptToExecuteOnDocumentCreatedAsync returns, the script is registered, so every exit before the ID reaches its member now removes it. Both the apartment hop (check_hresult'd, can throw) and the generation-mismatch Remove are handled explicitly. Without that, a throw between the Add and the member write would orphan the registration: the next call would read an empty ID, remove nothing, and Add a second bridge - and since kMessageBridgeScript is not IIFE-wrapped, the duplicate top-level `const originalPostMessage` would kill the second copy with a SyntaxError. Adds <cstdint> for uint32_t; no fixed-width integer type was used in these files before, so transitive availability is not relied on. UNCOMPILED: no Windows build, MSBuild, or lint was run against this repo.
|
Pushed the fix for the limitation I flagged in the PR body — What changedTrack the
Also stopped gating The part worth reviewing closelyOnce Both throwing calls in that window are now handled explicitly: One residual I'd rather state than hide: if that undo- Added Honest limits
Context@manage-some is independently testing this series on RNW 0.84.1 Fabric and has confirmed #3981 fixes the black-screen-on-relaunch. This addresses the post-mount-toggle gap I warned them about, so To be precise about what this does and doesn't buy, since it's easy to overstate: the |
|
Resolving the caveat I left in my last comment — whether the props reaching It's a complete snapshot, so the line is correct as written. The reason the question felt ambiguous is that both halves are true: the payload JS sends is a delta, but the props object handed to const auto *rawValue = rawProps.at(name, namePrefix, nameSuffix);
if (rawValue == nullptr) [[likely]] {
return sourceValue;
}
// Special case: `null` always means "the prop was removed, use default
// value".
if (!rawValue->hasValue()) [[unlikely]] {
return defaultValue;
}Two things follow directly:
Upstream of that, the props object is constructed by Consequence for this PR: an unrelated prop change cannot wipe the pre-content script. I'm deliberately not extending this to the other props in that function. Still unverified, unchanged from before: this is uncompiled — no Windows build or lint has run against this repo, and CI here has never run on this PR (five workflow runs have sat at |
|
Friendly bump — this has been open for a while without a decision. Is there anything you need from me to move it along (rebase onto current main, splitting it, extra tests)? Happy to update it either way. |
Part of the hardening series offered in #3980. This is WV-3: register the message bridge at document-start, and wire up
injectedJavaScriptBeforeContentLoaded.Problem
Two related timing bugs in the messaging path:
window.ReactNativeWebView(plus thealert()shim andpostMessageforwarding) is injected viaExecuteScriptAsyncfromOnNavigationCompleted— i.e. after the page has already finished loading.WebMessageReceivedis unhooked and re-hooked on everyOnNavigationStarting.Real-world failure mode
Any page script that reads or calls
window.ReactNativeWebViewwhile the page is still loading — a synchronous inline<script>in<head>, or a framework's early bootstrap code — seesundefinedand either throws or silently no-ops, because the bridge doesn't exist until afterNavigationCompletedfires. This is a real, commonly-hit ordering bug in embedded-web-content integrations generally (it's the reasoninjectedJavaScriptBeforeContentLoadedexists as a concept in this library on other platforms) — Windows has no "before content loaded" injection point at all today.Fix
kMessageBridgeScript(contents unchanged from the currentNavigationCompletedinjection — alert shim,postMessageforwarding,chrome.webviewmessage-event dispatch, all preserved) is registered once viaCoreWebView2.AddScriptToExecuteOnDocumentCreatedAsync, inOnCoreWebView2Initialized, instead of re-injected viaExecuteScriptAsyncon every completed navigation. This makes it exist before any page script runs, on every subsequent navigation in thatCoreWebView2.WebMessageReceivedis registered once at the same point instead of on everyNavigationStarting.AddScriptToExecuteOnDocumentCreatedAsyncmechanism now appliesinjectedJavaScriptBeforeContentLoaded(the prop already exists in the codegen'd spec —RCTWebView2Props::injectedJavaScriptBeforeContentLoaded— but nothing on Windows ever read it).Trade-off worth flagging
messagingEnabledandinjectedJavaScriptBeforeContentLoadedare now captured once, atCoreWebView2init, rather than re-checked on every navigation. PreviouslymessagingEnabledwas checked fresh in bothNavigationStartingandNavigationCompleted, so toggling it after mount took effect on the very next navigation; with this change it takes effect on remount instead. WebView2 doesn't offer a clean way to swap a document-created script after the fact short of tracking script IDs and callingRemoveScriptToExecuteOnDocumentCreated— happy to add that if the dynamic-toggle behavior matters for your users; flagging as a deliberate scope cut rather than an oversight.Validation status
Not compiled against this repo. No Windows build was run this pass — verified by hand against
RCTWebView2ComponentView.cpp, not by building.Document-start bridge registration via
AddScriptToExecuteOnDocumentCreatedAsync, andinjectedJavaScriptBeforeContentLoadedsupport built the same way, are both production-proven in a parallel, independently-built Windows new-arch WebView2 Fabric component running in Facilitron FIT (react-native-windows 0.83.2, WinAppSDK 1.8, ARM64) — see the production twin: FacilitronWorks/react-native-webview-windows. Renamed symbols, hand-ported to this repo's codegen/class shape — not a copy-paste.Related: #3980