From f0881be66e1d5c38078762399a0b053ed54f7cbe Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Tue, 27 Oct 2026 15:26:50 -0400 Subject: [PATCH 1/4] fix(android): keep other documents from replacing the editor A script in the editor could load blob, data, or new-window documents into the editor's frame, leaving the host with an editor that drops commands and fails content reads. Local documents now load in subframes only, new windows hand http(s) URLs to the browser, and any other page that still reaches the main frame is replaced by the editor again. Opening a URL no app handles no longer crashes. Co-Authored-By: Claude Opus 5.5 --- .../org/wordpress/gutenberg/GutenbergView.kt | 159 ++++++++++++---- .../gutenberg/GutenbergViewNavigationTest.kt | 175 +++++++++++++++++- 2 files changed, 292 insertions(+), 42 deletions(-) diff --git a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt index de18073e5..95eaaf2b0 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt @@ -1,6 +1,7 @@ package org.wordpress.gutenberg import android.annotation.SuppressLint +import android.content.ActivityNotFoundException import android.content.Context import android.content.Intent import android.graphics.Bitmap @@ -13,6 +14,7 @@ import android.os.Build import android.os.Bundle import android.os.Handler import android.os.Looper +import android.os.Message import android.security.NetworkSecurityPolicy import android.util.Log import android.view.Gravity @@ -137,6 +139,10 @@ class GutenbergView : FrameLayout { /** The editor document [loadEditor] loaded, the only page given the editor globals. */ private var editorUri: Uri? = null + + /** Web views created for new windows that have not navigated yet. */ + private val popupWebViews = mutableSetOf() + private val configuration: EditorConfiguration private lateinit var dependencies: EditorDependencies @@ -498,6 +504,9 @@ class GutenbergView : FrameLayout { @SuppressLint("SetJavaScriptEnabled") // Without JavaScript we have no Gutenberg private fun initializeWebView() { webView.settings.javaScriptCanOpenWindowsAutomatically = true + // Without this, `window.open()` and `target="_blank"` load into the editor's + // own frame. With it, they reach `onCreateWindow`. + webView.settings.setSupportMultipleWindows(true) webView.settings.javaScriptEnabled = true webView.settings.domStorageEnabled = true @@ -577,24 +586,11 @@ class GutenbergView : FrameLayout { override fun shouldOverrideUrlLoading(view: WebView?, request: WebResourceRequest): Boolean { val url = request.url - // Allow local file URLs - if (url.scheme == "file") { - return false - } - - // Allow blob URLs (used by block inserter) - if (url.scheme == "blob") { - return false - } - - // Allow data URLs (used by block inserter) - if (url.scheme == "data") { - return false - } - - // Allow about:blank URLs - if (url.scheme == "about") { - return false + // Local documents load in subframes, such as the block inserter's + // previews. In the main frame they would replace the editor, and no + // other app can open them. + if (url.scheme in LOCAL_DOCUMENT_SCHEMES) { + return request.isForMainFrame } // Allow asset URLs (restrict to the asset path prefix so that @@ -611,14 +607,26 @@ class GutenbergView : FrameLayout { // For all other URLs, open in external browser. This includes the site's // REST API: the editor reaches it by fetch, which never passes through here. - val intent = Intent(Intent.ACTION_VIEW, url) - intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) - view?.context?.startActivity(intent) + openExternally(url) return true } } webView.webChromeClient = object : WebChromeClient() { + override fun onCreateWindow( + view: WebView?, + isDialog: Boolean, + isUserGesture: Boolean, + resultMsg: Message? + ): Boolean { + val transport = resultMsg?.obj as? WebView.WebViewTransport ?: return false + // The URL of a new window is only known once it navigates, so the window + // is a hidden web view that hands that navigation to the browser. + transport.webView = createPopupWebView() + resultMsg.sendToTarget() + return true + } + override fun onConsoleMessage(consoleMessage: ConsoleMessage?): Boolean { if (consoleMessage != null) { Log.i("GutenbergView", consoleMessage.message()) @@ -681,6 +689,49 @@ class GutenbergView : FrameLayout { showErrorPhase(Exception(editorLoadErrorMessage(url, reason, isDevServer))) } + /** Opens [url] in another app, such as the browser, when one can open it. */ + private fun openExternally(url: Uri) { + val intent = Intent(Intent.ACTION_VIEW, url).addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) + try { + context.startActivity(intent) + } catch (e: ActivityNotFoundException) { + Log.w(TAG, "No app can open the ${url.scheme} URL", e) + } + } + + /** + * Creates a hidden web view for a new window. Its first navigation opens in the + * browser when it is http(s) and is dropped otherwise. + */ + private fun createPopupWebView(): WebView { + val popup = WebView(context) + popupWebViews.add(popup) + popup.webViewClient = object : WebViewClient() { + override fun shouldOverrideUrlLoading(view: WebView?, request: WebResourceRequest): Boolean { + closePopup(popup, request.url) + return true + } + + override fun onPageStarted(view: WebView?, url: String?, favicon: Bitmap?) { + // Catches loads `shouldOverrideUrlLoading` doesn't see. A new window + // is blank before its first navigation. + if (url == null || url == "about:blank") return + closePopup(popup, Uri.parse(url)) + } + } + return popup + } + + private fun closePopup(popup: WebView, url: Uri) { + // A navigation can reach both callbacks. + if (!popupWebViews.remove(popup)) return + if (url.scheme == "http" || url.scheme == "https") { + openExternally(url) + } + // The popup is still dispatching the callback that closes it. + handler.post { popup.destroy() } + } + /** * Fetches all required dependencies and then loads the editor. * @@ -795,9 +846,10 @@ class GutenbergView : FrameLayout { /** * Invoked when any page begins loading in the main frame. Resets readiness for - * every page; for the editor document alone, starts the upload server once — - * capturing the [mediaProcessor] provided before load — then advertises the - * editor globals (including the server's port and token). + * every page and loads the editor again over any other page; for the editor + * document alone, starts the upload server once — capturing the + * [mediaProcessor] provided before load — then advertises the editor globals + * (including the server's port and token). * * Starting the server here, on the UI thread, rather than from the * [mediaProcessor] setter keeps its whole lifecycle — start here, stop in @@ -810,11 +862,17 @@ class GutenbergView : FrameLayout { isEditorLoaded = false didFireEditorLoaded = false + if (url == null) return + // The globals carry the site credential and the upload server's token, so - // they go to the editor document alone. `shouldOverrideUrlLoading` admits - // other pages into this frame, and on Android the editor shares an origin - // with the site, so the destination is checked rather than assumed. - if (url == null || !isEditorDocument(Uri.parse(url))) return + // they go to the editor document alone. Some loads never pass + // `shouldOverrideUrlLoading` (POST forms, history, a host's `loadUrl`), and + // on Android the editor shares an origin with the site, so the destination + // is checked rather than assumed. + if (!isEditorDocument(Uri.parse(url))) { + if (editorUri != null) restoreEditorDocument() + return + } if (!hasStartedLoading) { hasStartedLoading = true @@ -823,6 +881,20 @@ class GutenbergView : FrameLayout { setGlobalJavaScriptVariables() } + /** + * Loads the editor again after another page replaced it. Hosts are told the + * editor is unavailable until it reports itself loaded. + */ + private fun restoreEditorDocument() { + Log.w(TAG, "Another page replaced the editor document, so the editor is reloading") + reloadEditor() + handler.post { + // Picks made in an open inserter can no longer reach the editor. + blockInserterDialog?.dismiss() + editorDidBecomeUnavailableListener?.onEditorUnavailable(this) + } + } + private fun setGlobalJavaScriptVariables() { val gbKit = GBKitGlobal.fromConfiguration( configuration, @@ -1012,23 +1084,23 @@ class GutenbergView : FrameLayout { } /** - * Notified when the editor crashes and is no longer usable. + * Notified when the editor crashes or is replaced and is no longer usable. * * This can be called without a preceding * [EditorAvailableListener.onEditorAvailable] when the editor crashes before * it finishes loading. * - * Either the editor's error boundary caught an error and React unmounted the - * editor, or the web view's renderer process died. Both remove every - * JavaScript `editor` API, so calls to them are refused from this point until - * the editor reloads. + * The editor's error boundary caught an error and React unmounted the editor, + * the web view's renderer process died, or another page replaced the editor + * document. Each removes every JavaScript `editor` API, so calls to them are + * refused from this point until the editor reloads. * * GutenbergKit either reloads the editor automatically after a renderer death - * or covers it with a notice offering to reload it. Until it reloads, hosts - * should disable the controls that depend on the editor — history, editor - * mode — while leaving those that read from their own persisted copy, such as - * saving and closing, available. Re-enable them the next time - * [EditorAvailableListener.onEditorAvailable] is called. + * or a replaced document, or covers it with a notice offering to reload it. + * Until it reloads, hosts should disable the controls that depend on the + * editor — history, editor mode — while leaving those that read from their own + * persisted copy, such as saving and closing, available. Re-enable them the + * next time [EditorAvailableListener.onEditorAvailable] is called. */ fun interface EditorUnavailableListener { fun onEditorUnavailable(view: GutenbergView?) @@ -1209,7 +1281,7 @@ class GutenbergView : FrameLayout { } /** - * Reloads the editor after it has crashed. + * Reloads the editor after it has crashed or another page replaced it. * * The reloaded editor starts from the content [LatestContentProvider] * returns, or from the content it was opened with when there is none. @@ -1217,6 +1289,7 @@ class GutenbergView : FrameLayout { * `onEditorLoaded` again. */ internal fun reloadEditor() { + val editorUrl = editorUri?.toString() ?: return isEditorLoaded = false hasRecoveredAutomatically = false // The reload replaces the page these reads were sent to, so their results @@ -1227,7 +1300,8 @@ class GutenbergView : FrameLayout { loadEditor(dependencies) } else { showSpinnerPhase() - webView.reload() + // `reload()` would reload whichever page now occupies the frame. + webView.loadUrl(editorUrl) } } } @@ -1515,6 +1589,8 @@ class GutenbergView : FrameLayout { errorView.animate().cancel() webView.animate().cancel() webView.stopLoading() + popupWebViews.forEach { it.destroy() } + popupWebViews.clear() contentChangeListener = null historyChangeListener = null saveAvailabilityListener = null @@ -1597,6 +1673,9 @@ class GutenbergView : FrameLayout { /** Hosts that are safe to serve assets over HTTP (local development only). */ private val LOCAL_HOSTS = setOf("localhost", "127.0.0.1", "10.0.2.2") + /** Schemes of documents the WebView creates or reads itself, without a server. */ + private val LOCAL_DOCUMENT_SCHEMES = setOf("about", "blob", "data", "file") + /** * Returns [url]'s origin authority: its host, plus its port when that port * is not the scheme's default. diff --git a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewNavigationTest.kt b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewNavigationTest.kt index ceb759abc..db81f5147 100644 --- a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewNavigationTest.kt +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewNavigationTest.kt @@ -1,9 +1,16 @@ package org.wordpress.gutenberg +import android.content.Intent import android.net.Uri +import android.os.Handler import android.os.Looper +import android.os.Message import android.webkit.WebResourceRequest +import android.webkit.WebView import kotlinx.coroutines.test.TestScope +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test @@ -44,10 +51,174 @@ class GutenbergViewNavigationTest { private fun editorUrlFor(siteUrl: String) = BuildConfig.GUTENBERG_EDITOR_URL .ifEmpty { "$siteUrl/assets/index.html" } - private fun opensExternally(view: GutenbergView, url: String): Boolean { + private fun request(url: String, isForMainFrame: Boolean = true): WebResourceRequest { val request = mock(WebResourceRequest::class.java) `when`(request.url).thenReturn(Uri.parse(url)) - return view.editorWebView.webViewClient.shouldOverrideUrlLoading(view.editorWebView, request) + `when`(request.isForMainFrame).thenReturn(isForMainFrame) + return request + } + + private fun opensExternally(view: GutenbergView, url: String): Boolean = + overridesLoading(view, url, isForMainFrame = true) + + /** Whether the editor's frame is kept from loading [url]. */ + private fun overridesLoading(view: GutenbergView, url: String, isForMainFrame: Boolean): Boolean = + view.editorWebView.webViewClient.shouldOverrideUrlLoading( + view.editorWebView, + request(url, isForMainFrame) + ) + + private fun nextStartedActivity(): Intent? = + shadowOf(RuntimeEnvironment.getApplication()).nextStartedActivity + + private fun idle() = shadowOf(Looper.getMainLooper()).idle() + + /** Opens a new window from the editor, returning the web view it loads into. */ + private fun openWindow(view: GutenbergView): WebView { + val webView = view.editorWebView + val transport = webView.WebViewTransport() + val message = Message.obtain(Handler(Looper.getMainLooper())).apply { obj = transport } + + val created = shadowOf(webView).webChromeClient + .onCreateWindow(webView, false, true, message) + + assertTrue("the editor should accept the new window", created) + return requireNotNull(transport.webView) { "the new window should have a web view" } + } + + @Test + fun `shouldOverrideUrlLoading keeps local documents out of the main frame`() { + val siteView = configuredSiteView() + listOf( + "blob:https://example.com/0b8f1e2a", + "data:text/html,

Not the editor

", + "about:blank", + "file:///android_asset/index.html" + ).forEach { url -> + assertTrue("$url should not replace the editor", overridesLoading(siteView, url, isForMainFrame = true)) + } + assertNull("no app is asked to open a local document", nextStartedActivity()) + } + + @Test + fun `shouldOverrideUrlLoading allows local documents in subframes`() { + // The block inserter previews patterns in `blob` and `data` iframes. + val siteView = configuredSiteView() + listOf( + "blob:https://example.com/0b8f1e2a", + "data:text/html,

Preview

", + "about:blank", + "file:///android_asset/index.html" + ).forEach { url -> + assertFalse("$url should load in a subframe", overridesLoading(siteView, url, isForMainFrame = false)) + } + } + + @Test + fun `shouldOverrideUrlLoading opens web pages in the browser`() { + val result = opensExternally(configuredSiteView(), "https://example.org/a-page") + + assertTrue("a web page should not replace the editor", result) + assertEquals( + "the browser should open the page", + Uri.parse("https://example.org/a-page"), + nextStartedActivity()?.data + ) + } + + @Test + fun `shouldOverrideUrlLoading tolerates URLs no app can open`() { + shadowOf(RuntimeEnvironment.getApplication()).checkActivities(true) + + val result = opensExternally(configuredSiteView(), "unknown-scheme://example") + + assertTrue("the URL should still be kept from the editor", result) + } + + @Test + fun `a new window opens web pages in the browser`() { + val popup = openWindow(configuredSiteView()) + + val result = popup.webViewClient.shouldOverrideUrlLoading(popup, request("https://example.org/preview")) + + assertTrue("the hidden window should not load the page", result) + assertEquals( + "the browser should open the page", + Uri.parse("https://example.org/preview"), + nextStartedActivity()?.data + ) + } + + @Test + fun `a new window drops local documents`() { + val popup = openWindow(configuredSiteView()) + + popup.webViewClient.onPageStarted(popup, "blob:https://example.com/0b8f1e2a", null) + + assertNull("no app is asked to open a local document", nextStartedActivity()) + } + + @Test + fun `a new window opens its page once when both callbacks see the navigation`() { + val popup = openWindow(configuredSiteView()) + + popup.webViewClient.shouldOverrideUrlLoading(popup, request("https://example.org/preview")) + popup.webViewClient.onPageStarted(popup, "https://example.org/preview", null) + + assertNotNull("the browser should open the page", nextStartedActivity()) + assertNull("the browser should open it only once", nextStartedActivity()) + } + + @Test + fun `a new window waits past its initial blank document`() { + val popup = openWindow(configuredSiteView()) + + popup.webViewClient.onPageStarted(popup, "about:blank", null) + popup.webViewClient.shouldOverrideUrlLoading(popup, request("https://example.org/preview")) + + assertEquals( + "the browser should open the page the window navigates to", + Uri.parse("https://example.org/preview"), + nextStartedActivity()?.data + ) + } + + @Test + fun `onPageStarted loads the editor again over a page that replaced it`() { + val siteView = configuredSiteView() + val webView = siteView.editorWebView + var unavailable = false + siteView.setEditorDidBecomeUnavailable { unavailable = true } + siteView.onEditorLoaded() + idle() + + // A host's `loadUrl` never passes `shouldOverrideUrlLoading`. + webView.loadUrl("https://example.com/a-page/") + webView.webViewClient.onPageStarted(webView, "https://example.com/a-page/", null) + idle() + + assertEquals( + "the editor document should replace the page", + editorUrlFor("https://example.com"), + shadowOf(webView).lastLoadedUrl + ) + assertTrue("the host should be told the editor is unavailable", unavailable) + } + + @Test + fun `reloadEditor loads the editor rather than the page that replaced it`() { + val siteView = configuredSiteView() + val webView = siteView.editorWebView + webView.loadUrl("https://example.com/a-page/") + + siteView.reloadEditor() + idle() + + assertEquals( + "the editor document should load", + editorUrlFor("https://example.com"), + shadowOf(webView).lastLoadedUrl + ) } @Test From cf5c83c99e27e737d16c04bb35bbed251aa28cb0 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 30 Oct 2026 15:45:47 -0400 Subject: [PATCH 2/4] fix(android): bound editor recovery and tolerate more window failures A script that navigates on every load reloaded the editor forever, closed windows stayed alive until the view detached, and an oversized URL crashed the app when opening it externally. Co-Authored-By: Claude Opus 5.5 --- .../org/wordpress/gutenberg/GutenbergView.kt | 48 ++++++++++++++---- .../gutenberg/GutenbergViewNavigationTest.kt | 50 ++++++++++++++++++- 2 files changed, 87 insertions(+), 11 deletions(-) diff --git a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt index 95eaaf2b0..42cb399f6 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt @@ -142,6 +142,7 @@ class GutenbergView : FrameLayout { /** Web views created for new windows that have not navigated yet. */ private val popupWebViews = mutableSetOf() + private var editorRestoreCount = 0 private val configuration: EditorConfiguration private lateinit var dependencies: EditorDependencies @@ -586,8 +587,8 @@ class GutenbergView : FrameLayout { override fun shouldOverrideUrlLoading(view: WebView?, request: WebResourceRequest): Boolean { val url = request.url - // Local documents load in subframes, such as the block inserter's - // previews. In the main frame they would replace the editor, and no + // Local documents load in subframes, such as Gutenberg's `Iframe` + // component. In the main frame they would replace the editor, and no // other app can open them. if (url.scheme in LOCAL_DOCUMENT_SCHEMES) { return request.isForMainFrame @@ -689,13 +690,18 @@ class GutenbergView : FrameLayout { showErrorPhase(Exception(editorLoadErrorMessage(url, reason, isDevServer))) } - /** Opens [url] in another app, such as the browser, when one can open it. */ + /** + * Opens [url] in another app, such as the browser, when one can open it. The + * page chooses the URL, so no URL may crash the host app: besides no app + * matching, the matched app can refuse the intent or the URL can exceed its limit. + */ + @Suppress("TooGenericExceptionCaught") private fun openExternally(url: Uri) { val intent = Intent(Intent.ACTION_VIEW, url).addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) try { context.startActivity(intent) - } catch (e: ActivityNotFoundException) { - Log.w(TAG, "No app can open the ${url.scheme} URL", e) + } catch (e: RuntimeException) { + Log.w(TAG, "Unable to open the ${url.scheme} URL", e) } } @@ -703,6 +709,7 @@ class GutenbergView : FrameLayout { * Creates a hidden web view for a new window. Its first navigation opens in the * browser when it is http(s) and is dropped otherwise. */ + @SuppressLint("SetJavaScriptEnabled") private fun createPopupWebView(): WebView { val popup = WebView(context) popupWebViews.add(popup) @@ -719,13 +726,20 @@ class GutenbergView : FrameLayout { closePopup(popup, Uri.parse(url)) } } + // Without JavaScript, `window.close()` never reaches `onCloseWindow`. + popup.settings.javaScriptEnabled = true + popup.webChromeClient = object : WebChromeClient() { + override fun onCloseWindow(window: WebView?) { + closePopup(popup, url = null) + } + } return popup } - private fun closePopup(popup: WebView, url: Uri) { + private fun closePopup(popup: WebView, url: Uri?) { // A navigation can reach both callbacks. if (!popupWebViews.remove(popup)) return - if (url.scheme == "http" || url.scheme == "https") { + if (url?.scheme == "http" || url?.scheme == "https") { openExternally(url) } // The popup is still dispatching the callback that closes it. @@ -886,8 +900,16 @@ class GutenbergView : FrameLayout { * editor is unavailable until it reports itself loaded. */ private fun restoreEditorDocument() { - Log.w(TAG, "Another page replaced the editor document, so the editor is reloading") - reloadEditor() + // A script that navigates on every load would otherwise reload forever. + if (editorRestoreCount < MAX_EDITOR_RESTORES) { + editorRestoreCount++ + Log.w(TAG, "Another page replaced the editor document, so the editor is reloading") + reloadEditor() + } else { + Log.e(TAG, "Another page keeps replacing the editor document, so the editor stopped reloading") + webView.stopLoading() + showEditorCrashPhase() + } handler.post { // Picks made in an open inserter can no longer reach the editor. blockInserterDialog?.dismiss() @@ -1275,7 +1297,10 @@ class GutenbergView : FrameLayout { titleResId = R.string.gbk_editor_crashed_title, descriptionResId = R.string.gbk_editor_crashed_description, actionResId = R.string.gbk_editor_crashed_reload, - onAction = { reloadEditor() } + onAction = { + editorRestoreCount = 0 + reloadEditor() + } ) } } @@ -1676,6 +1701,9 @@ class GutenbergView : FrameLayout { /** Schemes of documents the WebView creates or reads itself, without a server. */ private val LOCAL_DOCUMENT_SCHEMES = setOf("about", "blob", "data", "file") + /** How many times the editor reloads over other pages before asking the user to reload. */ + private const val MAX_EDITOR_RESTORES = 3 + /** * Returns [url]'s origin authority: its host, plus its port when that port * is not the scheme's default. diff --git a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewNavigationTest.kt b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewNavigationTest.kt index db81f5147..5a8cf0663 100644 --- a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewNavigationTest.kt +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewNavigationTest.kt @@ -1,5 +1,6 @@ package org.wordpress.gutenberg +import android.content.ContextWrapper import android.content.Intent import android.net.Uri import android.os.Handler @@ -102,7 +103,7 @@ class GutenbergViewNavigationTest { @Test fun `shouldOverrideUrlLoading allows local documents in subframes`() { - // The block inserter previews patterns in `blob` and `data` iframes. + // Gutenberg's `Iframe` component loads its document from a `blob` URL. val siteView = configuredSiteView() listOf( "blob:https://example.com/0b8f1e2a", @@ -135,6 +136,35 @@ class GutenbergViewNavigationTest { assertTrue("the URL should still be kept from the editor", result) } + @Test + fun `shouldOverrideUrlLoading tolerates an app refusing the URL`() { + val refusingContext = object : ContextWrapper(RuntimeEnvironment.getApplication()) { + override fun startActivity(intent: Intent) { + throw SecurityException("Permission denial") + } + } + val siteView = GutenbergView( + EditorConfiguration.builder("https://example.com", "https://example.com/wp-json/").build(), + EditorDependencies.empty, + testScope, + refusingContext + ) + + val result = opensExternally(siteView, "https://example.org/a-page") + + assertTrue("the URL should still be kept from the editor", result) + } + + @Test + fun `a new window closed by its script is discarded without opening anything`() { + val popup = openWindow(configuredSiteView()) + + shadowOf(popup).webChromeClient.onCloseWindow(popup) + popup.webViewClient.shouldOverrideUrlLoading(popup, request("https://example.org/preview")) + + assertNull("a closed window should not open the browser", nextStartedActivity()) + } + @Test fun `a new window opens web pages in the browser`() { val popup = openWindow(configuredSiteView()) @@ -205,6 +235,24 @@ class GutenbergViewNavigationTest { assertTrue("the host should be told the editor is unavailable", unavailable) } + @Test + fun `onPageStarted stops loading the editor again over a page that keeps replacing it`() { + val siteView = configuredSiteView() + val webView = siteView.editorWebView + val pageUrl = "https://example.com/a-page/" + val replaceEditor = { + webView.loadUrl(pageUrl) + webView.webViewClient.onPageStarted(webView, pageUrl, null) + idle() + shadowOf(webView).lastLoadedUrl + } + val editorUrl = editorUrlFor("https://example.com") + + repeat(3) { assertEquals("the editor should recover", editorUrl, replaceEditor()) } + + assertEquals("the editor should stop reloading", pageUrl, replaceEditor()) + } + @Test fun `reloadEditor loads the editor rather than the page that replaced it`() { val siteView = configuredSiteView() From 09659599f063bdca8f200e5812685dffde6ee64f Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 30 Oct 2026 15:57:16 -0400 Subject: [PATCH 3/4] fix(android): keep new windows from defeating renderer recovery Hidden new windows share the editor's renderer and left its death unhandled, which killed the app. Automatic reloads over a replacing page also re-armed the renderer recovery limit, which only the user's reload should do. Co-Authored-By: Claude Opus 5.5 --- .../org/wordpress/gutenberg/GutenbergView.kt | 31 ++++++++++++++----- .../gutenberg/GutenbergViewNavigationTest.kt | 17 ++++++++++ .../GutenbergViewRenderProcessGoneTest.kt | 17 ++++++++++ 3 files changed, 57 insertions(+), 8 deletions(-) diff --git a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt index 42cb399f6..4b349b682 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt @@ -725,6 +725,14 @@ class GutenbergView : FrameLayout { if (url == null || url == "about:blank") return closePopup(popup, Uri.parse(url)) } + + @RequiresApi(Build.VERSION_CODES.O) + override fun onRenderProcessGone(view: WebView?, detail: RenderProcessGoneDetail?): Boolean { + closePopup(popup, url = null) + // Popups share the editor's renderer, and any web view leaving this + // unhandled kills the app. + return true + } } // Without JavaScript, `window.close()` never reaches `onCloseWindow`. popup.settings.javaScriptEnabled = true @@ -904,7 +912,7 @@ class GutenbergView : FrameLayout { if (editorRestoreCount < MAX_EDITOR_RESTORES) { editorRestoreCount++ Log.w(TAG, "Another page replaced the editor document, so the editor is reloading") - reloadEditor() + loadEditorDocumentAgain() } else { Log.e(TAG, "Another page keeps replacing the editor document, so the editor stopped reloading") webView.stopLoading() @@ -1297,26 +1305,33 @@ class GutenbergView : FrameLayout { titleResId = R.string.gbk_editor_crashed_title, descriptionResId = R.string.gbk_editor_crashed_description, actionResId = R.string.gbk_editor_crashed_reload, - onAction = { - editorRestoreCount = 0 - reloadEditor() - } + onAction = { reloadEditor() } ) } } /** - * Reloads the editor after it has crashed or another page replaced it. + * Reloads the editor at the user's request, which also lets automatic recovery + * run again. + */ + internal fun reloadEditor() { + hasRecoveredAutomatically = false + editorRestoreCount = 0 + loadEditorDocumentAgain() + } + + /** + * Loads the editor document again, after it has crashed or another page + * replaced it. * * The reloaded editor starts from the content [LatestContentProvider] * returns, or from the content it was opened with when there is none. * Readiness is reset immediately and restored only once the editor emits * `onEditorLoaded` again. */ - internal fun reloadEditor() { + private fun loadEditorDocumentAgain() { val editorUrl = editorUri?.toString() ?: return isEditorLoaded = false - hasRecoveredAutomatically = false // The reload replaces the page these reads were sent to, so their results // may never arrive. failPendingTitleAndContentReads() diff --git a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewNavigationTest.kt b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewNavigationTest.kt index 5a8cf0663..265c81575 100644 --- a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewNavigationTest.kt +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewNavigationTest.kt @@ -6,6 +6,7 @@ import android.net.Uri import android.os.Handler import android.os.Looper import android.os.Message +import android.webkit.RenderProcessGoneDetail import android.webkit.WebResourceRequest import android.webkit.WebView import kotlinx.coroutines.test.TestScope @@ -165,6 +166,17 @@ class GutenbergViewNavigationTest { assertNull("a closed window should not open the browser", nextStartedActivity()) } + @Test + fun `a new window handles its renderer dying and is discarded`() { + val popup = openWindow(configuredSiteView()) + + val handled = popup.webViewClient.onRenderProcessGone(popup, mock(RenderProcessGoneDetail::class.java)) + popup.webViewClient.shouldOverrideUrlLoading(popup, request("https://example.org/preview")) + + assertTrue("an unhandled renderer death kills the app", handled) + assertNull("a discarded window should not open the browser", nextStartedActivity()) + } + @Test fun `a new window opens web pages in the browser`() { val popup = openWindow(configuredSiteView()) @@ -251,6 +263,11 @@ class GutenbergViewNavigationTest { repeat(3) { assertEquals("the editor should recover", editorUrl, replaceEditor()) } assertEquals("the editor should stop reloading", pageUrl, replaceEditor()) + + siteView.reloadEditor() + idle() + + assertEquals("the user's reload should allow recovery again", editorUrl, replaceEditor()) } @Test diff --git a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewRenderProcessGoneTest.kt b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewRenderProcessGoneTest.kt index 4dde7561d..bbac8e3ff 100644 --- a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewRenderProcessGoneTest.kt +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewRenderProcessGoneTest.kt @@ -155,6 +155,23 @@ class GutenbergViewRenderProcessGoneTest { assertEquals("the crash notice covers the editor", View.INVISIBLE, view.editorWebView.visibility) } + @Test + fun `an automatic reload over a replacing page does not allow another automatic recovery`() { + val view = loadedSiteView() + killRenderer(view) + view.onEditorLoaded() + shadowOf(Looper.getMainLooper()).idle() + + val webView = view.editorWebView + webView.webViewClient.onPageStarted(webView, "https://example.com/a-page/", null) + shadowOf(Looper.getMainLooper()).idle() + view.onEditorLoaded() + shadowOf(Looper.getMainLooper()).idle() + killRenderer(view) + + assertNull("only the user's reload allows another recovery", shadowOf(view.editorWebView).lastLoadedUrl) + } + @Test fun `reloading from the crash notice allows another automatic recovery`() { val view = loadedSiteView() From 804158c18ed19cd214b62da73f744cfc8edd6070 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 30 Oct 2026 16:05:57 -0400 Subject: [PATCH 4/4] fix(android): report open dialogs closed when another page replaces the editor No editor mounts behind the error view shown after repeated replacements, so nothing reported an open dialog closed. Hosts kept Close disabled and Back was refused, leaving Reload Editor as the only way out. Co-Authored-By: Claude Opus 5.5 --- .../org/wordpress/gutenberg/GutenbergView.kt | 15 +++++++++---- .../gutenberg/GutenbergViewNavigationTest.kt | 21 +++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt index 4b349b682..c26c7325b 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt @@ -921,10 +921,20 @@ class GutenbergView : FrameLayout { handler.post { // Picks made in an open inserter can no longer reach the editor. blockInserterDialog?.dismiss() + reportOpenModalDialogsClosed() editorDidBecomeUnavailableListener?.onEditorUnavailable(this) } } + /** + * Reports the editor's open dialogs closed after it went away without unmounting + * them, rather than leave the host's navigation blocked. + */ + private fun reportOpenModalDialogsClosed() { + openModalDialogs.forEach { modalDialogStateListener?.onModalDialogClosed(it) } + openModalDialogs.clear() + } + private fun setGlobalJavaScriptVariables() { val gbKit = GBKitGlobal.fromConfiguration( configuration, @@ -1363,10 +1373,7 @@ class GutenbergView : FrameLayout { failPendingTitleAndContentReads() replaceWebView() blockInserterDialog?.dismiss() - // The dead renderer can't unmount the editor's open dialogs to report - // them closed, so report them here rather than leave navigation blocked. - openModalDialogs.forEach { modalDialogStateListener?.onModalDialogClosed(it) } - openModalDialogs.clear() + reportOpenModalDialogsClosed() editorDidBecomeUnavailableListener?.onEditorUnavailable(this) if (wasEditorLoaded && !hasRecoveredAutomatically) { hasRecoveredAutomatically = true diff --git a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewNavigationTest.kt b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewNavigationTest.kt index 265c81575..e2d2f7c87 100644 --- a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewNavigationTest.kt +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewNavigationTest.kt @@ -247,6 +247,27 @@ class GutenbergViewNavigationTest { assertTrue("the host should be told the editor is unavailable", unavailable) } + @Test + fun `onPageStarted reports open dialogs closed when another page replaces the editor`() { + val siteView = configuredSiteView() + val webView = siteView.editorWebView + val closed = mutableListOf() + siteView.setModalDialogStateListener(object : GutenbergView.ModalDialogStateListener { + override fun onModalDialogOpened(dialogType: String) = Unit + + override fun onModalDialogClosed(dialogType: String) { + closed += dialogType + } + }) + siteView.onModalDialogOpened("block-inspector") + idle() + + webView.webViewClient.onPageStarted(webView, "https://example.com/a-page/", null) + idle() + + assertEquals("the host would keep its navigation disabled", listOf("block-inspector"), closed) + } + @Test fun `onPageStarted stops loading the editor again over a page that keeps replacing it`() { val siteView = configuredSiteView()