Let the document follow the app into night mode again - #596
Merged
Conversation
The redesign replaced `PageView.toggleDarkMode(enabled)` with `disableDarkening()`, called on every page. The shell kept following night mode and the document stayed white, which readers noticed and wrote in about. Darkening is allowed for every format, pdf included, until we have looked at enough of them to say which ones invert badly. `DocumentFragment` is the one place that is decided. `PrintingManager` takes its `onFinished` callback back, so printing can turn darkening off and put it back once the job ends - the print framework reads the page long after `print()` returns. `DarkModeTests` puts the app in night mode and asserts three things: the flag, the setting the webview was actually given, and the mean luminance of the screen. All three fail against the old behaviour, the screenshot one with "the page stayed light - mean luminance 255". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TouAQNfktsp9THcceennEX
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c27efa4fdb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
`FORCE_DARK_AUTO` is the platform's smart dark, which an app declaring a dark theme is deliberately left out of - so on a webview old enough to have only that api the page never darkened, which is what `theDrawnPageIsDark` was failing on at API 29 and 30. It is `FORCE_DARK_ON` now, asked only when the app itself reports night mode, which is the part AUTO cannot work out for us. Where the webview has neither api - API 26's is too old - the screenshot test skips rather than asserts a page nothing could have darkened. From the review of #596: - printing waited for the print *job* to reach a terminal state, so a queued or blocked job left the document light for as long as the printer stayed off. The framework lets go of the adapter long before that, so `PrintDocumentAdapter .onFinish` is what restores it now, with the job poll left as the backstop for the adapter that is dropped without a last call. Whichever comes first wins, once. - `MainActivity` snapshotted `isDarkeningAllowed` around the print, so a second print started while the first was still spooling snapshotted the suspended value and one of the two restores went the wrong way. `PageView` counts the suspensions instead and only darkens again once the last reader is done, and the page it is given back to is looked up again rather than captured. `DarkModeTests.printingGivesTheDarkeningBack` holds the counting. All 73 instrumented tests pass on an API 29 emulator, where three of them failed before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4Tbjr2eTpVkBxDfdFnkgF
andiwand
enabled auto-merge (squash)
August 18, 2026 06:09
This was referenced Aug 18, 2026
Merged
andiwand
added a commit
that referenced
this pull request
Aug 18, 2026
The engine moves from 6.7.1 to 6.8.0, and nothing here has to move with it. The format table gains a `color_scheme` capability per row and nothing else - no type, extension or mime type changes - so `SupportedDocumentTypes` derives the same two sets and `STRICT_CATCH`'s generated intent-filters still match them, which `SupportedFormatsTest` confirms. `FileTypeCapabilities` and `HtmlConfig` gain a field each, both additive, and `CoreLoader` names the ones it sets. 6.8.0's headline is the one thing this app already answers itself. The core can now render a file in the dark, on `HtmlConfig.colorScheme`, which defaults to `LIGHT` and writes nothing at all in that case - no `color-scheme` declaration reaches the page, so the algorithmic darkening `PageView` turns on keeps working exactly as it did in #596. Handing that job to the core instead is worth doing, since it would let a document's own colours give way rather than be inverted, and it is a change of its own rather than part of taking the version. What reaches a user is text this time. A megabyte of it took 38 seconds to lay out, because sizing the line numbers laid the page out once per line, and now takes under a second. Prose is no longer read as a csv: a separator every field follows with a space, in fields long enough to be sentences, is punctuation, while `a, b, c` with short values is still a table. And text reflowed to the viewport is inset 3mm from the screen edge rather than starting at its first pixel. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
andiwand
added a commit
that referenced
this pull request
Aug 18, 2026
Two readers wrote in about the same screen from opposite ends: one wanted the document to follow the phone into night mode, the other wanted the margins off without going back to the landing screen for them. Both answers were somewhere else - one in the system settings, one behind a fold on another screen - so the document's own buttons carry them now. Three rows, each saying what tapping it does rather than what it is called: - **Night mode** puts the app in it through `AppCompatDelegate.setLocalNightMode` rather than the default mode, so reading at night no longer means turning the whole phone dark first. It recreates the activity the way a rotation does and survives it the same way: the loader is a ViewModel, the fragment saves the document, and the page view its own state. `NightModeSetting` remembers the choice - a local mode does not outlive the process - and stores no override at all once the answer agrees with the system again, or the app would sit in night mode through a morning the phone had long left. - **Darkening** is per kind of document rather than one answer for everything. A text document inverts into something that still reads; a scanned page or a photograph inverts into something nobody wrote. So `DocumentDarkening` defaults documents to dark and pdfs and images to light, and the button edits that for every document of the kind - it is never *this* pdf that inverts badly, it is pdfs. Google Docs forgets the same choice on reopen and Word remembers it for everything at once; per kind is the middle, and the row names the kind it will remember so the memory is not invisible. It applies live: darkening is a webview setting, not something the page was translated with. This reverses `aPdfIsDarkenedToo` from #596, which included every format deliberately, "until we have looked at enough of them to say which ones invert badly". Acrobat is the one that has looked: its night mode is opt-in per document. Presentations and drawings stay with documents, because that much is still a guess. The kind comes from the core's own table - `fileTypeByMimetype`, then the file type itself, never a prefix. `documentTypeByFileType` cannot answer it: application/pdf reports TEXT, exactly like an odt. - **The margins** are odrcore's `textDocumentMargin`, decided while translating, so the button renders the document again through the reload that edit mode already uses - the copy in the cache, which is not read or re-listed a second time. The landing switch stays: it writes the same preference, so the two can never disagree. Fullscreen moves up beside them, as the third row about what the page looks like rather than what can be done to it. `DarkModeTests` gains the switch darkening a day-mode app (the screen is measured, not the flag) and the per-kind answer surviving a reopen; `MainActivityTests` that the margin row renders the document again rather than only flipping a preference. All 76 pass on an API 36 emulator. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VNSUAUN1QX2QyFNw4rAmV2
andiwand
added a commit
that referenced
this pull request
Aug 18, 2026
* Answer how a document is displayed over the document itself Two readers wrote in about the same screen from opposite ends: one wanted the document to follow the phone into night mode, the other wanted the margins off without going back to the landing screen for them. Both answers were somewhere else - one in the system settings, one behind a fold on another screen - so the document's own buttons carry them now. Three rows, each saying what tapping it does rather than what it is called: - **Night mode** puts the app in it through `AppCompatDelegate.setLocalNightMode` rather than the default mode, so reading at night no longer means turning the whole phone dark first. It recreates the activity the way a rotation does and survives it the same way: the loader is a ViewModel, the fragment saves the document, and the page view its own state. `NightModeSetting` remembers the choice - a local mode does not outlive the process - and stores no override at all once the answer agrees with the system again, or the app would sit in night mode through a morning the phone had long left. - **Darkening** is per kind of document rather than one answer for everything. A text document inverts into something that still reads; a scanned page or a photograph inverts into something nobody wrote. So `DocumentDarkening` defaults documents to dark and pdfs and images to light, and the button edits that for every document of the kind - it is never *this* pdf that inverts badly, it is pdfs. Google Docs forgets the same choice on reopen and Word remembers it for everything at once; per kind is the middle, and the row names the kind it will remember so the memory is not invisible. It applies live: darkening is a webview setting, not something the page was translated with. This reverses `aPdfIsDarkenedToo` from #596, which included every format deliberately, "until we have looked at enough of them to say which ones invert badly". Acrobat is the one that has looked: its night mode is opt-in per document. Presentations and drawings stay with documents, because that much is still a guess. The kind comes from the core's own table - `fileTypeByMimetype`, then the file type itself, never a prefix. `documentTypeByFileType` cannot answer it: application/pdf reports TEXT, exactly like an odt. - **The margins** are odrcore's `textDocumentMargin`, decided while translating, so the button renders the document again through the reload that edit mode already uses - the copy in the cache, which is not read or re-listed a second time. The landing switch stays: it writes the same preference, so the two can never disagree. Fullscreen moves up beside them, as the third row about what the page looks like rather than what can be done to it. `DarkModeTests` gains the switch darkening a day-mode app (the screen is measured, not the flag) and the per-kind answer surviving a reopen; `MainActivityTests` that the margin row renders the document again rather than only flipping a preference. All 76 pass on an API 36 emulator. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Let the core say what darkens, and keep the reader's place Three answers the review asked for, and one the new engine can give. odrcore 6.8.0 renders a file dark itself, so `CoreLoader` translates every page with `HtmlColorScheme.SYSTEM`: a format that has a dark of its own now carries both, each behind the media query the webview answers, and the colours the file authored give way instead of being inverted. Nothing about the button changes - `PageView.setDarkeningAllowed` still picks between the two at display time, so darkening is still a webview setting and still renders nothing again. That makes `capabilitiesByFileType(...).colorScheme` the answer to what should darken, which `DocumentDarkening` had been guessing at. It defaults to that rather than to a list, and the guesses are gone: an image darkens now, because the core puts a dark ground under the photograph rather than into it, while a pdf and the media views have no dark of their own and are left as they were, offered but off. `Kind` is only what an override is remembered for. The margin button is offered where it does something. odrcore applies `textDocumentMargin` to a text document and nothing else, so on a pdf, an image or a spreadsheet the row translated the document again to show exactly the same page - and answered for the next text document opened while it was at it. Four types call themselves text and one of them is pdf, which is fixed pages laid out by a frontend the margin never reaches, so `PaginationSetting.affects` excludes it by name. And the margins reload keeps the reader where they were. It is the one thing that throws the page away and translates it again, and it came back at the top of the first tab; `DocumentFragment` now carries the tab and how far down it over to the document that arrives. A fraction rather than the offset, because the margins are exactly what changed the height it would have been measured against, and applied once the layout stops growing rather than when the load reports itself finished - a long document is still being laid out then. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Darken a file the core could not name, as before `darkensByDefault` asked the core about a file type and refused when there was none, which is a mime type the core does not know - and that file is being shown as text or as the html fallback, both of which have a dark of their own. It is also the kind `kindOf` gives it. The list this replaced said DOCUMENT and so darkened it; this now says the same. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Say what the webview was, when a page will not go dark These tests fail on one api level at a time, on emulators whose webview is far older than any developer machine's, and "the page stayed light - mean luminance 252" does not say which of the two darkening apis was in play, or whether the page view was allowed to darken at all. Both luminance assertions now say. Nothing about the app changes. `theSwitchDarkensADayModeApp` fails on api 29 and nowhere else, it fails the same before and after this branch's darkening changes, and the first guess at why - that the restored page was rendered before darkening reached it - was wrong: moving the call ahead of `restore` changed the reading not at all. So this asks the failing run instead of guessing again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Invert on the old webview, rather than stand aside for a theme it cannot match The diagnosis the last commit added says what api 29 is: webview 74, no algorithmic darkening, force dark instead - and the app doing its part, the page view allowed to darken and force dark switched on, over a page that stayed light. Force dark defaults to standing aside for a page carrying a dark theme of its own, and since `HtmlColorScheme.SYSTEM` every page carries one. A webview this old answers prefers-color-scheme by the system alone and never matches it for a forced dark, so what it stands aside for is nothing at all. Asking for `DARK_STRATEGY_USER_AGENT_DARKENING_ONLY` says invert it anyway, which on that branch is the only dark there is; the newer branch is untouched and still gets the page's own dark, which is the better one. This is not the whole of api 29: main fails `theDrawnPageIsDark` there too, at the same luminance and without any of this branch's changes, so that webview was already not darkening reliably. What this fixes is the part this branch would otherwise have added to it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Skip the pixel tests on a webview that cannot darken, and wait for the text Two levels were red for reasons of their own. Api 29 ships webview 74, which reports `FORCE_DARK` supported, takes the setting, hands it back, and draws the page exactly as light as before - force dark is only implemented from 76. So `canDarken` asks the version as well as the api, and the two tests that read pixels skip there rather than fail for a webview that was never going to darken. What the app does is still asserted on api 29, through `darkeningSetting`: allowed, and the setting it was given. A version that cannot be read counts as capable, since a skip taken by mistake is coverage quietly lost. `aMegabyteOfTextOpensAndIsSearchable` waited for the element count to stop changing and then searched, and found nothing. A megabyte is parsed in bursts, so two equal readings can both land inside one pause: the page had laid out, which is what that wait asserts, and was still filling. It now waits for the text as well - every line carries the needle once, so they are all in the page or it is not all there yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Hold the reading position the margin switch used to lose Polled with a timeout and reported with what it actually read, the way the edit mode tests do it: the page has to be laid out again before there is anywhere to put anyone back to, so there is nothing to assert straight after the reload. A fraction on both sides. The margins are exactly what changes the height, so the same place in the text is a different offset once the page has been laid out again - hence a tolerance rather than an equality. It scrolls to the middle rather than the end, since the last screenful is one position however far past it the page is scrolled, and a test that stopped there would pass without anything being restored at all. `style-various-1.docx` because it is long enough to scroll, and a text document, which is the only kind the margin button is offered on. With the restore taken back out it fails with "the reader was put back at 0.0 of the page, not around 0.5", which is what it is for. `verticalScrollableHeight` is public now because the test cannot reach the protected webview call it was spelled out of, and it was written three times in here anyway. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Say the same thing about the display buttons in fewer words Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Trim the test comments and the note in CLAUDE.md to what they have to say Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Do not let a page that was navigated away from fail the one that replaced it `testDOCXEditMode` and `testODTEditMode` have been failing an api level at a time all week, always with "webview did not answer in 10000ms document=no result" - and `no result` is the tell: `lastDocument` is only null once something called `unload`, so the document had not merely failed to become editable, it had been given up on. Logcat from a failing run says what did it. A page that never commits leaves a reload scheduled 2.5s out - the workaround for a webview reporting progress 100 over a blank page - and nothing cancelled it when the next page was asked for. The test before had deliberately loaded a page that 404s, so its retry fired inside the next test, went back to a server that had gone with the document that owned it, and got a 404. `onReceivedHttpError` reported that against whatever document was on screen by then, which was the docx being opened for edit mode. So `loadUrl` cancels a retry the page before it left waiting, the way `destroy` already does when the whole view is replaced. And `failPage` answers only for the page it was last asked to load: a request made for a document already closed can still be answered long after, and the document on screen is not the one that failed. Nothing is loosened. `LandingTests.aDocumentThatFailsToOpenComesBackToTheList` holds the case that matters - a document whose page really is a 404 - and the whole instrumented suite, 80 tests, passes locally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Check the retry against the page still wanted, not only the flag Cancelling on `loadUrl` clears the retries queued until then, and not the one queued after: page A can finish - and schedule its retry - once B has already been asked for. `wasCommitCalled` is about whichever page is being waited on, so if B has not committed within the 2.5s, A's retry reads B's flag, believes it is about itself, and loads A back over B. Which is the navigation this set out to stop, arrived at from the other side. So the retry asks whether the page it names is still the page wanted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
From user feedback: "Kein Dark Mode mehr wie vorher?"
The redesign (#547) replaced
PageView.toggleDarkMode(enabled)withdisableDarkening(), called on every page. The shell kept following night mode; only the document stayed white. In night mode the app was a dark frame around a white page.The change
setDarkeningAllowed(allowed), called once perPageViewinDocumentFragment.initializePageView(). It is on for every format, pdf included — the pre-redesign code excluded pdf, but rather than guess which formats invert badly we can look at a spread of real documents in the simulator and cut it back from that one line.Darkening only ever does anything while the app itself is in night mode: at targetSdk 33+ the WebView darkens algorithmically and only once the app theme reports itself dark.
PrintingManagertakes itsonFinishedcallback back, so printing can turn darkening off and put it back once the job ends — the print framework reads the page long afterprint()returns, so it cannot be restored inline.Tests
DarkModeTestsputs the app in night mode and checks three things, weakest to strongest:WebSettingsCompat.isAlgorithmicDarkeningAllowed)The third is the one that matters: a WebView that ignored the setting would pass the first two. It has to be a screenshot rather than
pageView.draw()into a bitmap — the WebView renders on the compositor, so drawing it into a bitmap of our own hands back an empty layer whatever the page looks like. That version passed against the broken behaviour, which is how it was caught.All three fail against the old behaviour, the screenshot one with
the page stayed light - mean luminance 255.Verified
spotlessCheck,testProDebugUnitTest,lintProDebug,assembleDebug, and the full instrumented suite (72 tests) green on a Pixel 9 Pro emulator.On a short landscape AVD (
ODR_Screenshots, 1216x535)MainActivityTests.testPasswordProtectedODTfails — it does so 3/3 onmainas well, so it is that AVD and not this branch.🤖 Generated with Claude Code