Serve a limited ad to whoever refuses consent, instead of nothing - #569
Serve a limited ad to whoever refuses consent, instead of nothing#569andiwand wants to merge 2 commits into
Conversation
Refusing everything got no ad at all. That was a choice rather than an obligation: "do not consent" still emits a tc string carrying the special purposes, and google picks limited ads from it server-side - no cookies, no identifiers, no local storage. There is no client-side flag for that mode, so not sending the request is the only way to lose it. So canRequestAds() stops being a gate, which is where this parts ways with google's own sample. The two consent errors stop being gates for the same reason - neither is a refusal. The sdk caches the user's decision, so a form that fails to show, or an update that times out because the device was asleep, still leaves an earlier consent standing, and outside the regions where a form is required at all there is no decision to fail. One timeout used to blank the banner for the rest of the session, for users who had already said yes. The rest follows from asking for ads we now expect not to fill: - Limited ads fill at a fraction of the rate, and there was no AdListener at all - the container was made visible before the load resolved. Nothing enters it until the listener fires now, and a request that does not fill puts a house ad for Pro there rather than an empty strip. Three angles at the same offer, one per request, sized to the banner they replace; the subline drops below 360dp and the icon below 300dp, and neither line wraps, so a long translation shortens instead of breaking the height. The sdk retries behind our back and reports every attempt, hence the guard against walking through all three texts in half a second. - The consent form needs a re-entry point, both because withdrawal has to be as easy as granting and because the tcf asks for one. It goes in the overflow menu, gated on the sdk's own privacyOptionsRequirementStatus - which is only answerable once the update has come back, so the menu is invalidated then. Not gated on the ad manager being enabled: buying ad removal clears that, and someone who consented before buying must still be able to take it back. - requestConsentInfoUpdate stays at once per launch, which is what refreshes region and message version. onConfigurationChanged used to re-run the whole thing; it now only rebuilds the banner, whose size is what the orientation actually changes. Verified on device: consent declined -> an ad still serves; offline -> the consent error is logged and the house ad appears rather than silence; the privacy item shows on first launch and reopens the form; all three house ad texts cycle, and both drop-out widths render. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0141xLyuaHVh7rizEVDb8q75
5f9a759 to
4051678
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4051678e2e
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Adjusts the Lite flavor ad/consent flow so users who refuse consent can still receive Google’s server-selected limited ads (instead of no ad request and an empty banner), while also improving UX around unfilled requests and consent re-entry.
Changes:
- Always load an ad banner after the UMP consent flow settles (even on consent update / form errors), enabling limited ads for “reject all”.
- Add a “house ad” banner UI to display when an ad request doesn’t fill, instead of showing an empty strip.
- Add an overflow-menu entry to reopen privacy options when UMP indicates it’s required, and avoid re-running consent updates on rotation.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/res/values/strings.xml | Adds menu label and house-ad copy strings. |
| app/src/main/res/values/colors.xml | Introduces house-ad-specific colors. |
| app/src/main/res/menu/menu_main.xml | Adds hidden-by-default privacy options menu item. |
| app/src/main/res/layout/house_ad.xml | New layout for the fallback “house ad” banner. |
| app/src/main/res/drawable/house_ad_pill.xml | CTA pill drawable for house ad. |
| app/src/main/res/drawable/house_ad_background.xml | Background + divider drawable for house ad banner. |
| app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt | Wires privacy menu invalidation + purchase callback and refreshes ads on rotation. |
| app/src/main/java/app/opendocument/droid/nonfree/AdManager.kt | Reworks consent flow gating, adds privacy options support, and introduces house-ad fallback on no-fill. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Three points from the review of the parent commit. An update that fails on a device that has never had a consent decision leaves the sdk with nothing: no tc string, so the request google would get carries nothing to pick limited ads from. That is not the case this branch argues for, so `hasConsentDecision` gates the request on the sdk holding an answer - a refusal counts, `UNKNOWN` does not - and the house ad, which is where an unfilled request ends anyway, goes up directly. Ad removal used to take the withdrawal item with it: `showGoogleAds` is what populated `consentInformation`, and a purchase means it never runs. `updateConsentInfo` is the consent update without a form or a banner behind it, which is all `isPrivacyOptionsRequired` needs, and the purchased path asks for it. Wherever ads never ran at all - the pro flavor, the instrumented suite - it is a no-op. Every rotation built an `AdView` and dropped the previous one out of the container without destroying it, webview and all. The outgoing one is destroyed now, and a callback from a banner a rotation has replaced is ignored rather than allowed to overwrite the current one. Comments throughout are shorter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NdFTJsRNpeYhXfN6GpcL1p
Refusing all consent got no ad at all. Per
ad-consent-notes.md, that is a choice rather than an obligation:"Do not consent" still emits a TC string carrying the special purposes, and Google picks limited ads from it server-side: no cookies, no identifiers, no local storage. There is no client-side flag for that mode, so not sending the request is the only way to lose it.
What changed
canRequestAds()stops being a gate — which is where this parts ways with Google's own sample, deliberately. The two consent errors stop being gates for the same reason: neither is a refusal. The SDK caches the user's decision, so a form that fails to show, or an update that times out because the device was asleep, still leaves an earlier consent standing; outside the regions where a form is required at all there is no decision to fail. One timeout used to blank the banner for the rest of the session, for users who had already said yes.The rest follows from asking for ads we now expect not to fill:
A house ad instead of an empty strip. There was no
AdListenerat all, and the container was made visible before the load resolved. Nothing enters it until the listener fires now, and a request that does not fill puts a Pro house ad there. Three angles at the same offer, one per request, sized to the banner they replace — the subline drops below 360dp and the icon below 300dp, and neither line wraps, so a long translation shortens instead of breaking the height.A re-entry point into the consent form, in the overflow menu, gated on the SDK's own
privacyOptionsRequirementStatus. Withdrawal has to be as easy as granting (GDPR Art. 7(3)) and the TCF asks for one. The status is only answerable once the update has come back, so the menu is invalidated then. Not gated on the ad manager being enabled: buying ad removal clears that, and someone who consented before buying must still be able to take it back.requestConsentInfoUpdatestays at once per launch, which is what refreshes region and message version.onConfigurationChangedused to re-run the whole thing; it now only rebuilds the banner, whose size is what the orientation actually changes.The call this rests on
"Programmatic limited ads" (AdMob → Settings → Account information) is on, the default. Off means waterfall mediation only and near-zero fill, and this change would buy nothing. On means Google uses invalid-traffic-detection-only cookies and local storage on unconsented traffic. Google's position is that no consent is needed for that; the legal responsibility is the publisher's. Worth stating plainly in a privacy-forward FOSS app: it is a deliberate call, not an oversight.
Pro/IAP ad removal remains the clean answer for "neither ads nor tracking", and the F-Droid build covers the absolutist case.
Why it is worth fixing
Android match rate — the share of ad requests Google fills — since the consent form shipped in
f6b942b6(Dec 2023):Show rate over the same period improved to ~92%, so this is not a rendering problem. Most of that gap is expected and correct — non-personalised requests are simply filled less often. What this change adds back is the reject cohort, which was being discarded entirely, plus the requests lost to a consent call that merely failed.
Testing
Built and driven on an emulator (lite debug), against a
DEBUG_GEOGRAPHY_EEAoverride that was removed before committing:consent info update failed: 2/…in logcat followed by the house ad, rather than by silence. This is the case the PR originally fixed, preserved.invalidateMenu()fires; reopens the form; absent in the pro flavor, where the consent update never runs.requestConsentInfoUpdate.spotlessCheck,lintProDebug,lintLiteDebug,testProDebugUnitTestand bothassemblevariants pass. No test covers any of this and none can:MainActivity.IS_TESTINGforcesadManager.setEnabled(false)whenever the instrumented suite is on the classpath.Still open, elsewhere
From the notes, not addressable in this repo: the iOS UMP/CMP work and its ordering before ATT, the IDFA explainer message in the AdMob UI, and the TCF v2.3 / serving-restriction check.
🤖 Generated with Claude Code