From 3f988c1749460528c31dfc239a3a9544a8f52c30 Mon Sep 17 00:00:00 2001 From: Peter Stone Date: Mon, 12 Oct 2026 00:41:24 +0000 Subject: Stop reader mode passing a site's cookie notice off as the article Reported live: reader mode on a New Republic feature showed nothing but the cookie disclaimer, and had cached it. Reproduced outside the app by running ReaderExtractor's own injected script (Readability.js and all) against the real page. With the article body present, extraction is perfect. With the body absent from the DOM at the moment the script runs, Readability's first pass -- which discards "unlikely candidates", and the Didomi banner's class is didomi-notice-banner -- comes back under its own 500-character threshold, so it retries with that stripping switched off. The banner's single paragraph is 510 characters, i.e. just over the threshold, so it comes back as the article with the right title attached. Nothing on the Kotlin side looked at that. Any non-blank content counted as ExtractionResult.Success, was written to articles.extractedContent, and was never reconsidered -- so one bad load became a permanent cookie disclaimer, which is the "extractedContent cached: true" in the report. ConsentNotice recognises such a result: a hard plain-text length cap first (nothing long is ever rejected, however much consent boilerplate it quotes), then either a known CMP container id -- Readability strips classes but keeps ids -- or two or more distinct consent phrases. The accept/retry/give-up decision moves out of the WebView callback into decideEvalOutcome so it can be tested without a device. A consent notice is now "not ready yet", which routes it into the retry the extractor already does for an empty result; if it survives all six attempts that is what the page serves, and it fails as ExtractionFailure.ConsentInterstitial rather than being accepted. A fixed extractor cannot repair what is already on disk, so loadCachedExtraction drops a cached notice on read and re-extracts, in both ReaderScreen and LinkReaderScreen. No schema change: the repair happens when the article is next opened. Reader mode is still never turned on heuristically -- replacing a discarded notice does not flip it. The bug report now also carries the last extraction error, without which a report about bad reader content cannot say whether extraction failed. Tests: 59 -> 78. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B --- .../org/terst/feedreader/data/CachedExtraction.kt | 41 ++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 app/src/main/java/org/terst/feedreader/data/CachedExtraction.kt (limited to 'app/src/main/java/org/terst/feedreader/data/CachedExtraction.kt') diff --git a/app/src/main/java/org/terst/feedreader/data/CachedExtraction.kt b/app/src/main/java/org/terst/feedreader/data/CachedExtraction.kt new file mode 100644 index 0000000..313d467 --- /dev/null +++ b/app/src/main/java/org/terst/feedreader/data/CachedExtraction.kt @@ -0,0 +1,41 @@ +package org.terst.feedreader.data + +import org.terst.feedreader.readability.ConsentNotice + +/** What a cached `articles.extractedContent` value turned out to be worth. */ +data class CachedExtraction( + /** The content to show, or null if there is nothing usable. */ + val content: String?, + /** + * True when a cached consent notice was thrown away. The caller has nothing to + * show and should extract again -- the cache is the reason the article looked + * broken, so dropping it silently would just leave a blank reader. + */ + val discardedConsentNotice: Boolean +) + +/** + * Reads a cached extraction, dropping it if it is the site's cookie/consent notice + * rather than the article. + * + * Needed because the extractor's own guard can only keep a bad extraction from + * being written in future. The reported bug ("reader mode just extracts cookie + * disclaimer", with `extractedContent cached: true`) was already on disk: every + * subsequent open read it straight back out, so no amount of fixing the extractor + * would have changed what the user saw. A poisoned row is cleared here rather than + * in a schema migration, so it is repaired whenever the article is next opened -- + * including for anything extracted before this check existed. + * + * [clearCache] is passed in rather than taking the DAO so this stays a plain + * decision that can be tested without a database. + */ +suspend fun loadCachedExtraction( + cached: String?, + clearCache: suspend () -> Unit +): CachedExtraction { + if (!ConsentNotice.looksLikeConsentNotice(cached)) { + return CachedExtraction(content = cached, discardedConsentNotice = false) + } + clearCache() + return CachedExtraction(content = null, discardedConsentNotice = true) +} -- cgit v1.2.3