From 32709b04f9f2f115e8255a044905d6612cd20c55 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Fri, 14 Aug 2026 20:19:42 +0200 Subject: [PATCH 1/2] Take the server's error page back to the landing screen The core translates a page on the server thread, long after CoreLoader reported success, so a rendering failure only reaches the app as a status code. onReceivedError never sees one - the server did answer - so nobody detected it and the WebView rendered odrcore's "Internal Server Error" page in place of the document. PageView now overrides onReceivedHttpError as well, and both it and onReceivedError take the main frame through failPage(). DocumentFragment ends it where a document that would not open ends: back on the landing screen with the contact dialog. This also covers a server that never bound, which comes back as ERR_CONNECTION_REFUSED. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EVNWZNEjtiodTCP8kvTfNP --- .../droid/test/MainActivityTests.kt | 38 +++++++++++++++++++ .../droid/ui/activity/DocumentFragment.kt | 35 +++++++++++++++++ .../opendocument/droid/ui/widget/PageView.kt | 36 ++++++++++++++++-- 3 files changed, 105 insertions(+), 4 deletions(-) diff --git a/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt b/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt index f415ba854256..a0f7ba4bd686 100644 --- a/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt +++ b/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt @@ -175,6 +175,33 @@ class MainActivityTests { onView(withText(R.string.action_contact)).check(matches(isDisplayed())) } + /** A page the server answers with an error is not a document, whatever the load reported. */ + @Test + fun aPageTheServerCannotServeOffersContact() { + val activity = mainActivityActivityTestRule.activity + val documentFragment = loadDocument(activity, requireTestFile("test.odt")) + + val partUri = documentFragment.lastDocument?.partUris?.firstOrNull() + Assert.assertNotNull("no page was published", partUri) + + val pageView = documentFragment.pageView + Assert.assertNotNull(pageView) + + // a path the server has nothing under, which errors the same way a page it cannot render + InstrumentationRegistry.getInstrumentation().runOnMainSync { + pageView!!.loadUrl("$partUri.missing") + } + + // the webview reports the error long after loadUrl returns, and nothing idles on it + Assert.assertTrue( + "the page that failed was not given up on", + waitFor(10000) { documentFragment.lastDocument == null }, + ) + + onView(withText(R.string.dialog_broken_file)).check(matches(isDisplayed())) + onView(withText(R.string.action_contact)).check(matches(isDisplayed())) + } + @Test fun testODTEditMode() { val activity = mainActivityActivityTestRule.activity @@ -330,6 +357,17 @@ class MainActivityTests { return fragment } + private fun waitFor(timeoutMs: Long, condition: () -> Boolean): Boolean { + val startMs = SystemClock.elapsedRealtime() + do { + if (condition()) { + return true + } + SystemClock.sleep(100) + } while (SystemClock.elapsedRealtime() - startMs < timeoutMs) + return false + } + private fun waitForDocumentFragment( activity: MainActivity, timeoutMs: Long, diff --git a/app/src/main/java/app/opendocument/droid/ui/activity/DocumentFragment.kt b/app/src/main/java/app/opendocument/droid/ui/activity/DocumentFragment.kt index 6a43c709fafe..80fd9b697209 100644 --- a/app/src/main/java/app/opendocument/droid/ui/activity/DocumentFragment.kt +++ b/app/src/main/java/app/opendocument/droid/ui/activity/DocumentFragment.kt @@ -627,6 +627,41 @@ class DocumentFragment : Fragment(), DocumentLoader.Listener { state.endLoadIdling() } + /** + * The page [PageView] was given cannot be shown after all, so this ends where a document that + * would not open ends: back on the landing screen, offering to tell us about it. + * + * It arrives after [onLoadSuccess] rather than instead of it, which is why it undoes it instead + * of going through [isActivityReadyForOutcome]. + */ + fun onPageFailed() { + // already given up on, or a second view of the same failure + if (state.lastDocument == null) { + return + } + + if (activity == null || isStateSaved) { + replayOnStart = { onPageFailed() } + + return + } + + val activity = requireActivity() + + analyticsManager.report( + "page_failed", + AnalyticsConstants.PARAM_CONTENT_TYPE, + state.lastFile?.mimeType, + ) + + unload() + dismissProgress() + + giveUp(activity) + + offerContact(activity) + } + override fun onEncrypted(request: DocumentRequest, file: IdentifiedFile) { if (!isActivityReadyForOutcome(request, file) { onEncrypted(request, file) }) { return diff --git a/app/src/main/java/app/opendocument/droid/ui/widget/PageView.kt b/app/src/main/java/app/opendocument/droid/ui/widget/PageView.kt index f021947c92db..b2bb64c76fd9 100644 --- a/app/src/main/java/app/opendocument/droid/ui/widget/PageView.kt +++ b/app/src/main/java/app/opendocument/droid/ui/widget/PageView.kt @@ -13,6 +13,7 @@ import android.util.Base64InputStream import android.webkit.JavascriptInterface import android.webkit.WebResourceError import android.webkit.WebResourceRequest +import android.webkit.WebResourceResponse import android.webkit.WebView import android.webkit.WebViewClient import androidx.annotation.Keep @@ -101,10 +102,30 @@ constructor(context: Context, attributeSet: AttributeSet?) : return } - crashManager.log( - RuntimeException( - "loading ${request.url} failed: ${error.errorCode} ${error.description}" - ) + failPage( + "loading ${request.url} failed: ${error.errorCode} ${error.description}" + ) + } + + /** + * An error status is the only shape a rendering failure has here: the core + * translates a page on the server thread, long after `CoreLoader` reported success. + * [onReceivedError] never sees it - the server did answer. + */ + override fun onReceivedHttpError( + view: WebView, + request: WebResourceRequest, + errorResponse: WebResourceResponse, + ) { + super.onReceivedHttpError(view, request, errorResponse) + + if (!request.isForMainFrame) { + return + } + + failPage( + "serving ${request.url} failed: " + + "${errorResponse.statusCode} ${errorResponse.reasonPhrase}" ) } @@ -180,6 +201,13 @@ constructor(context: Context, attributeSet: AttributeSet?) : super.destroy() } + /** Reports a page that will never appear. [description] is all there is of the cause. */ + private fun failPage(description: String) { + crashManager.log(RuntimeException(description)) + + documentFragment.onPageFailed() + } + /** Whether [url] is a document we produced: a cached file, or the core's own http server. */ private fun isOwnContent(url: String): Boolean = url.startsWith("file://") || url.startsWith(LOCAL_SERVER_URL_PREFIX) From 7a3dcf409fa108084983f69779d5d79c11049106 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Fri, 14 Aug 2026 20:28:42 +0200 Subject: [PATCH 2/2] Give up on the document, not on a link that failed shouldOverrideUrlLoading returns false when it finds no app for a link, which leaves the webview to try the navigation itself - and that failure arrives as a main frame error too. failPage() now only gives up when the url is our own content, so a dead hyperlink no longer presents the open document as damaged. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EVNWZNEjtiodTCP8kvTfNP --- .../droid/test/MainActivityTests.kt | 23 +++++++++++++++++++ .../opendocument/droid/ui/widget/PageView.kt | 14 ++++++++--- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt b/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt index a0f7ba4bd686..177fed3338bc 100644 --- a/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt +++ b/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt @@ -202,6 +202,29 @@ class MainActivityTests { onView(withText(R.string.action_contact)).check(matches(isDisplayed())) } + /** A link that fails is not the document failing, and must not be reported as one. */ + @Test + fun aLinkThatFailsLeavesTheDocumentOpen() { + val activity = mainActivityActivityTestRule.activity + val documentFragment = loadDocument(activity, requireTestFile("test.odt")) + + val pageView = documentFragment.pageView + Assert.assertNotNull(pageView) + + // what the webview is left to try when shouldOverrideUrlLoading finds no app for a link + InstrumentationRegistry.getInstrumentation().runOnMainSync { + pageView!!.loadUrl("nosuchscheme://example") + } + + // a plain wait: this asserts something does not happen, after a webview round trip + // that nothing idles on + SystemClock.sleep(3000) + InstrumentationRegistry.getInstrumentation().waitForIdleSync() + + Assert.assertNotNull("the document was given up on", documentFragment.lastDocument) + onView(withText(R.string.dialog_broken_file)).check(doesNotExist()) + } + @Test fun testODTEditMode() { val activity = mainActivityActivityTestRule.activity diff --git a/app/src/main/java/app/opendocument/droid/ui/widget/PageView.kt b/app/src/main/java/app/opendocument/droid/ui/widget/PageView.kt index b2bb64c76fd9..f5f1f59d1f92 100644 --- a/app/src/main/java/app/opendocument/droid/ui/widget/PageView.kt +++ b/app/src/main/java/app/opendocument/droid/ui/widget/PageView.kt @@ -103,7 +103,8 @@ constructor(context: Context, attributeSet: AttributeSet?) : } failPage( - "loading ${request.url} failed: ${error.errorCode} ${error.description}" + request.url, + "loading ${request.url} failed: ${error.errorCode} ${error.description}", ) } @@ -124,8 +125,9 @@ constructor(context: Context, attributeSet: AttributeSet?) : } failPage( + request.url, "serving ${request.url} failed: " + - "${errorResponse.statusCode} ${errorResponse.reasonPhrase}" + "${errorResponse.statusCode} ${errorResponse.reasonPhrase}", ) } @@ -202,9 +204,15 @@ constructor(context: Context, attributeSet: AttributeSet?) : } /** Reports a page that will never appear. [description] is all there is of the cause. */ - private fun failPage(description: String) { + private fun failPage(url: Uri, description: String) { crashManager.log(RuntimeException(description)) + // only the document is the app's to give up on. a link shouldOverrideUrlLoading could not + // hand to another app is left to the webview, and fails here as a main frame load too + if (!isOwnContent(url.toString())) { + return + } + documentFragment.onPageFailed() }