Fix weekly review regressions

[kasnder]

Summary

• honor explicit per-app tracker-protection choices in Minimal mode, so the bulk exclude/include actions work for ordinary apps as well as browsers
• preserve VPN recovery after onRevoke() only when Android still designates TrackerControl as the always-on VPN
• restore the missing Slovenian Mullvad-country error message and its %s placeholder
• add regression coverage for tracker-protection defaults/overrides and VPN revoke behavior

Root causes and impact

Minimal mode previously returned true for every non-browser app before consulting the stored per-package preference. The bulk “Exclude all apps” action therefore appeared to succeed while most apps stayed protected.

The revoke handler kept the global enabled preference set for every revoke. Since the watchdog restarts an enabled service when its VPN descriptor is absent, a deliberate system-settings revoke or another VPN taking the slot could cause TrackerControl to reclaim it. The new behavior retains recovery only for a still-configured always-on VPN.

The Slovenian msg_wg_mullvad_countries_failed override was empty and omitted the formatting placeholder, producing a blank error and failing Android lint.

Validation

• ./gradlew --no-daemon testFdroidDebugUnitTest lintFdroidDebug
• git diff --check

[kasnder]

Verdict: Two of the three fixes (Slovenian string, tracker-protection defaults) are correct, complete, and well-tested. The onRevoke() change is the substantive one: it reverses a heavily-documented prior decision, and its correctness hinges on undocumented isAlwaysOn()/onRevoke() timing — validate on-device before merge.

Should fix

• ServiceSinkhole.java:onRevoke() (~L3211-3240 in diff): #526 recovery ("always-on VPN turns itself off after 3-5 days") now depends entirely on isAlwaysOn() returning true during the transient OS teardown/re-auth revoke. The ordering of when Android clears/reasserts the always-on designation relative to the onRevoke() callback is undocumented; if it reads false mid-teardown, resolveEnabledAfterRevoke persists enabled=false and TC self-disables permanently with no recovery — the exact failure the removed comment was written to prevent. There is no test or device evidence that the flag holds. Suggest validating a real #526-style always-on teardown on-device, and/or only clearing enabled when you positively observe another package holding the always-on slot rather than treating "not always-on" as authoritative. (unverified)
• ServiceSinkhole.java:onRevoke() pre-Q branch: minSdk is 23, so API 23-28 devices take the Settings.Secure.getString(cr, "always_on_vpn_app") path. That key is a @hide setting; on some OEM/OS builds it can return null even when TC is the configured always-on VPN, which disables TC on every revoke for those users. Confirm the key is reliably readable, or treat null as "unknown → keep enabled" instead of "not always-on → disable." (unverified)

Nits

• The stated goal of honoring a deliberate system-settings disable symmetrically depends on isAlwaysOn() returning false at revoke time; if the always-on designation lingers momentarily, the watchdog reclaims the slot and TC comes back on — defeating the intent. Worth an inline note that both directions rest on the same timing assumption.
• VpnRevokePolicyTest uses RobolectricTestRunner to exercise a pure boolean function. That is justified (loading ServiceSinkhole's static footprint under plain JUnit would throw), but the test only touches resolveEnabledAfterRevoke — no behavior change needed, just flagging it as heavier than it looks.

What's good

• BlockingMode.resolveTrackerProtection (L88-98) is a clean, surgical fix: verified against the old isMinimalMode(c) || getBoolean(pkg, true) logic, every no-explicit-pref default is preserved (non-browser → on, browser → off) and only the minimal-mode case that silently overrode an explicit false is corrected — so bulk "Exclude all apps" now sticks. Covered by a tidy truth-table test.
• Slovenian msg_wg_mullvad_countries_failed now carries the %s placeholder matching the base string (values/strings.xml:233), fixing both the blank message and the lint failure.
• Both behavioral fixes ship with focused regression tests, and the extracted static helpers make the policy testable without Android state.

Automated review by Claude Opus 4.8 (agent-assisted).

[kasnder]

Review update (2026-07-11): mostly ready, but validate the revoke policy on a real device before merging. CI is green and the Minimal-mode preference fix plus its regression coverage look correct. The follow-up also correctly treats an unreadable pre-Q always-on setting as unknown instead of forcibly disabling the service.

The remaining risk is Android timing: correctness still assumes the always-on designation is authoritative inside onRevoke(). Please exercise both directions on-device: (1) an OS/transient revoke while TC remains always-on must recover, and (2) disabling TC in system VPN settings or replacing it with another VPN must not be reclaimed by the watchdog. If those pass, merge this PR; if they cannot be verified, split/merge the tracker-protection fix separately and keep the revoke change out until tested.