Preserve per-site desktop mode preferences#8877
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…ed from the get go
RealSitePreferencesRepository now implements two interfaces (SitePreferencesRepository and MainProcessLifecycleObserver), so @ContributesBinding can no longer infer the bound type. Specify it explicitly to fix the DI codegen failure in both AnvilDagger and Metro builds (kaptGenerateStubs / Metro compile). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| // Persist the choice per-site. uri.host is the pre-rewrite host (e.g. m.example.com), whose eTLD+1 | ||
| // (example.com) matches the key the desktop-rewritten URL produces; hosts without a registrable | ||
| // domain (IPs, localhost) fall back to the raw host. The optimistic cache update means the reload | ||
| // triggered below already sees the new value. |
There was a problem hiding this comment.
nit: I'd recommend shorter comments or even skip them if code is clear
| Entity::class, | ||
| Relation::class, | ||
| DefaultBrowserPromptsAppUsageEntity::class, | ||
| SitePreferencesEntity::class, |
There was a problem hiding this comment.
Have you maybe considered creating a new module for site-preferences and a separate database? ;)
Asking because this relates to our Modularization AOI and adding new features that use AppDatabase will make it harder later to remove them. I'm not saying you need to do the new module now, just something to keep in mind.
(cc @aibrahim- )
There was a problem hiding this comment.
Do we really need a room DB for this? it seems a simple preferences store should be enough, e.g. androidx.datastore or even a SharedPreferences
There was a problem hiding this comment.
@aibrahim- It seemed like a natural choice, since the settings is per-domain, which can grow indefinitely. Plus, this would allow to store other prefs per-domain in the future. But I supposed storing it as a list of domains in a DataStore would work if the list was reasonable.
There was a problem hiding this comment.
@landomen Yeah, I suppose it could be a separate module. I can refactor it.
# Conflicts: # app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt # app/src/test/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt
…ore the site-preferences cache is primed Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ntracts Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… remember-desktop-mode flag Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ase to version 61 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…sktop-mode read Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@landomen @aibrahim- I moved the site preferences to a separate module and tried to address the issue you noticed. It is now better but a site set to desktop mode occasionally still renders mobile because the desktop-UA reload is served from the HTTP cache, which can return a previously-fetched mobile copy (a forced refresh fixes it). This is a pre-existing limitation not introduced here, it is just easier to surface now that desktop mode is applied automatically on every initial load of a remembered site. |
…rTest so the mocked listener returns false instead of a null Boolean
|
@landomen Thanks! For sure, just wanted you to take a look first to make sure it made sense. Proposal: https://app.asana.com/1/137249556945/project/1207418217763355/task/1215944643003573?focus=true |
# Conflicts: # app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt # app/src/test/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt
3 participants
Task/Issue URL: https://app.asana.com/1/137249556945/project/1214749231578703/task/1212541545715038?focus=true
API Proposal: https://app.asana.com/1/137249556945/project/1207418217763355/task/1215944643003573?focus=true
Description
This PR adds a per-site desktop mode preference persistence using eTLD+1 as a key (or "localhost" or an IP address). When a desktop mode is enabled on a site, it will be always used until it's either disabled or the site is cleared using the Fire button.
Steps to test this PR
Note
Medium Risk
Touches core WebView navigation (user-agent / intercept path) and new persistent local data tied to fire/clear semantics; well-tested but regressions could affect desktop vs mobile rendering per site.
Overview
Adds a new
site-preferencesmodule (Room-backed store) so “Desktop site” can be remembered per site key (eTLD+1, or raw host for localhost/IPs), gated by therememberDesktopModeremote feature toggle.Browser behavior: Toggling desktop/mobile in the menu writes or clears that preference;
WebViewRequestInterceptornow callsisDesktopSiteEnabled(url)(suspend) so user-agent selection follows the stored choice on load.pageChangedsyncs in-tab UI from the in-memory cache so desktop mode does not leak across domains.Data clearing: Fire / clear-browser flows call
clearAllButFireproofed(aligned with fireproof sites); per-domain burn alsoforgetDesktopModefor cleared domains.When the feature is off, behavior stays the previous session-only in-tab desktop flag.
Reviewed by Cursor Bugbot for commit 9e8594a. Bugbot is set up for automated code reviews on this repo. Configure here.