Repository navigation
fix(android): skip Cordova lifecycle events when the page has no bridge - #8638
Draft
theproducer wants to merge 1 commit into
Draft
theproducer wants to merge 1 commit into
theproducer wants to merge 1 commit into
Conversation
MockCordovaWebViewImpl.triggerDocumentEvent evaluated window.Capacitor.triggerEvent(...) unconditionally. The document in the WebView is not guaranteed to carry the bridge: the injected script is scoped to the app origin, so an appAllowNavigation origin, the server.errorPath page (WebViewLocalServer.handleLocalRequest returns the error page without running the JS injector) and a load still in flight all run without window.Capacitor. Backgrounding or foregrounding the app on such a page raised an uncaught TypeError inside the page, surfacing only as: Capacitor/Console: ... Uncaught TypeError: Cannot read properties of undefined (reading 'triggerEvent') Guard the call in the evaluated snippet and log a debug line when the event is dropped. Pause/resume on a loaded page is unchanged. This is the Android counterpart of #8595, which guarded the iOS scene lifecycle events on the page's load state. Covered by a new instrumented test that reproduces the console error before the change and asserts the events still reach a page that does have the bridge. Closes #8637
This branch has not been deployed
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
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.
Root cause
MockCordovaWebViewImpl.triggerDocumentEvent(String)(android/capacitor/src/main/java/com/getcapacitor/cordova/MockCordovaWebViewImpl.java) evaluatedwith no check that the page holds the bridge.
Bridge.onPause()/Bridge.onResume()always forward to this (the only guard isisInitialized(), which is true for the whole life of the activity), so a pause or resume reaches whatever document the WebView currently holds.That document is not guaranteed to have
window.Capacitor:Bridge.loadWebView()registers the bridge script withWebViewCompat.addDocumentStartJavaScript(webView, …, Collections.singleton(allowedOrigin)), whereallowedOriginis derived fromappUrlonly. Any other origin the WebView navigates to — e.g. anappAllowNavigationhost used for an OAuth hand-off — runs without the bridge. This is the shape reported in Breaking [Bug]: Capacitor is undefined, Capacitor.triggerEvent is not a function in allowNavigation routes #7957.server.errorPathpage.WebViewLocalServer.handleLocalRequestreturns the error page from theisLocalFile(...) || isErrorUrl(...)branch, which never callsjsInjector.getInjectedStream(...). Note this only strips the bridge on a WebView withoutDOCUMENT_START_SCRIPT, or whenserver.urlputs the app on a different origin from the error page — on a current WebView with noserver.urlthe error page is same-origin withappUrland does get the script.The resulting TypeError is raised inside the page, so it only surfaces through
BridgeWebChromeClient.onConsoleMessageas theCapacitor/Consoleline in the report. No functional breakage beyond the lost Cordovapause/resumeon a page that could not have consumed them anyway.The change
Guard the call inside the evaluated snippet and log a debug line when the event is dropped. Dropping rather than deferring matches #8595: a page without the bridge has nothing to pause or resume, and a deferred
resumeafter a fresh load would make the page react to a transition it never went through.pause/resumeon a loaded app page are unchanged.The snippet stays ES5 (
window.Capacitor && window.Capacitor.triggerEvent) rather than optional chaining, becauseandroid.minWebViewVersionallows WebViews older than the Chrome 80 that introduced?..How it was tested
New instrumented test
android/capacitor/src/androidTest/java/com/getcapacitor/android/CordovaLifecycleEventTest.java, on a Pixel 9a API 36 emulator:skipsLifecycleEventsWhenBridgeIsAbsent— loadsabout:blankinto the bridge's WebView (assertingtypeof window.Capacitor === 'undefined'first, so the test cannot pass vacuously), pumpsonPause()/onResume(), and fails if the console records atriggerEventerror. Before the change it fails with the exact reported string:After the change it passes, and Logcat shows
D Capacitor: Skipped the 'pause' document event: no Capacitor bridge on the current page.stillDeliversLifecycleEventsWhenBridgeIsPresent— installs awindow.Capacitor.triggerEventstub and assertspause:documentandresume:documentboth still arrive.Suites run:
./gradlew lint build test -b capacitor/build.gradle— BUILD SUCCESSFUL, "Lint found no new issues".adb am instrument—OK (5 tests)(CordovaLifecycleEventTest×2,HttpInterceptorNavigationTest×2,ExampleInstrumentedTest).prettierwith@ionic/prettier-config— both touched files clean.connectedAndroidTestcould not be used directly because the UTP host dependencies were not in the local Gradle cache; the APK was built with Gradle and driven withadb am instrumentinstead, which runs the sameAndroidJUnitRunner.What a reviewer should check by hand
pause/resume— the test asserts this against a JS stub, not againstcore/native-bridge.ts's realtriggerEvent.server.errorPathpath withserver.urlset, which is the configuration where the error page is genuinely cross-origin from the bridge script. I traced it but did not build that configuration.Bridge.triggerJSEvent(...)(Bridge.java:887-893), which has the identical unguarded pattern. It is public API called explicitly by plugins rather than by the lifecycle, so I left it alone — say the word and I will follow up.Related
window.Capacitorundefined onallowNavigationroutes; the same missing-bridge condition, with a reproduction.cap run androidblank screen, “Cannot read property ‘triggerEvent’ of undefined” #7048, TypeError: window.Capacitor.triggerEvent is not a function. (In 'window.Capacitor.triggerEvent('pause', 'document')', 'window.Capacitor.triggerEvent' is undefined) #7458 — earlier reports of the same console string, closed without the cause being established.