Don't show location permission message in SERP#7879
Don't show location permission message in SERP#7879
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Dev application ID accidentally committed in build config
- Reverted applicationId to com.duckduckgo.mobile.android, restored debug applicationIdSuffix, and changed icons back from gold to blue.
- ✅ Fixed: SERP test mocks wrong dependency, tests wrong path
- Changed the SERP test to mock mockDuckDuckGoUrlDetector.isDuckDuckGoUrl() instead of mockDuckChat.isDuckChatUrl(), and used a SERP URL with query parameter to differentiate from the plain DDG URL test.
- ✅ Fixed: Accidentally committed test with generic name
- Removed the leftover test() function from DuckDuckGoUrlDetectorTest as it duplicated existing coverage and had a non-descriptive name.
Or push these changes by commenting:
@cursor push 32dd8929eb
Preview (32dd8929eb)
diff --git a/app/build.gradle b/app/build.gradle
--- a/app/build.gradle
+++ b/app/build.gradle
@@ -22,8 +22,7 @@
android {
defaultConfig {
-// applicationId "com.duckduckgo.mobile.android"
- applicationId "com.duckduckgo.dev.android"
+ applicationId "com.duckduckgo.mobile.android"
minSdk min_sdk
targetSdk target_sdk
@@ -140,11 +139,11 @@
}
buildTypes {
debug {
-// applicationIdSuffix ".debug"
+ applicationIdSuffix ".debug"
pseudoLocalesEnabled false
manifestPlaceholders = [
- appIcon : "@mipmap/ic_launcher_gold",
- appIconRound: "@mipmap/ic_launcher_gold_round"
+ appIcon : "@mipmap/ic_launcher_blue",
+ appIconRound: "@mipmap/ic_launcher_blue_round"
]
}
release {
diff --git a/app/src/test/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/test/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt
--- a/app/src/test/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt
+++ b/app/src/test/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt
@@ -3666,15 +3666,15 @@
@Test
fun whenUserVisitsSERPUrlWithPermanentLocationPermissionThenMessageIsNotShown() =
runTest {
- val domain = "https://duckduckgo.com/"
+ val domain = "https://duckduckgo.com/?q=test"
whenever(mockSitePermissionsManager.hasSitePermanentPermission(domain, LocationPermissionRequest.RESOURCE_LOCATION_PERMISSION))
.thenReturn(true)
- whenever(mockDuckChat.isDuckChatUrl(any())).thenReturn(true)
+ whenever(mockDuckDuckGoUrlDetector.isDuckDuckGoUrl(domain)).thenReturn(true)
givenCurrentSite(domain)
- loadUrl("https://duckduckgo.com/", isBrowserShowing = true)
+ loadUrl("https://duckduckgo.com/?q=test", isBrowserShowing = true)
assertCommandNotIssued<Command.ShowDomainHasPermissionMessage>()
}
diff --git a/app/src/test/java/com/duckduckgo/app/browser/DuckDuckGoUrlDetectorTest.kt b/app/src/test/java/com/duckduckgo/app/browser/DuckDuckGoUrlDetectorTest.kt
--- a/app/src/test/java/com/duckduckgo/app/browser/DuckDuckGoUrlDetectorTest.kt
+++ b/app/src/test/java/com/duckduckgo/app/browser/DuckDuckGoUrlDetectorTest.kt
@@ -162,9 +162,4 @@
assertEquals("images", vertical)
}
- @Test
- fun test() {
- val isDDG = testee.isDuckDuckGoUrl("https://duckduckgo.com/")
- assertTrue(isDDG)
- }
}| defaultConfig { | ||
| applicationId "com.duckduckgo.mobile.android" | ||
| // applicationId "com.duckduckgo.mobile.android" | ||
| applicationId "com.duckduckgo.dev.android" |
There was a problem hiding this comment.
Dev application ID accidentally committed in build config
High Severity
The applicationId was changed from com.duckduckgo.mobile.android to com.duckduckgo.dev.android, the original is commented out, the debug applicationIdSuffix is commented out, and icons were changed from blue to gold. These look like local development configuration changes that were accidentally included in this PR. If merged, this would change the app's identity, preventing existing installations from receiving it as an update.
Additional Locations (1)
| loadUrl("https://duckduckgo.com/", isBrowserShowing = true) | ||
|
|
||
| assertCommandNotIssued<Command.ShowDomainHasPermissionMessage>() | ||
| } |
There was a problem hiding this comment.
SERP test mocks wrong dependency, tests wrong path
Medium Severity
The test whenUserVisitsSERPUrlWithPermanentLocationPermissionThenMessageIsNotShown mocks mockDuckChat.isDuckChatUrl(any()) to return true, but the test is supposed to verify SERP (DuckDuckGo URL) detection. It passes because the isDuckChatUrl check short-circuits the condition !isDuckChatUrl && !isDuckDuckGoUrl, not because isDuckDuckGoUrl returns true. The mock for isDuckDuckGoUrl is missing, so this test doesn't actually validate the SERP-specific logic it claims to test.
| fun test() { | ||
| val isDDG = testee.isDuckDuckGoUrl("https://duckduckgo.com/") | ||
| assertTrue(isDDG) | ||
| } |
There was a problem hiding this comment.
Accidentally committed test with generic name
Low Severity
A test function named simply test() was added to DuckDuckGoUrlDetectorTest. This appears to be a leftover debug/exploratory test — it has a non-descriptive name and duplicates coverage already provided by whenCheckingFullDDGUrlThenIdentifiedAsDDGUrl. It looks like it was accidentally included in the commit.
1 participant
Task/Issue URL: https://app.asana.com/1/137249556945/project/414730916066338/task/1213541802817811
Description
Don't show location permission message in SERP
Steps to test this PR
Location enabled - SERP
Location enabled - Other sites
Note
Medium Risk
Behavior changes around permission messaging and tab closing are user-visible and touch browser navigation state. Additionally, the
applicationId/debug packaging changes inapp/build.gradlecould affect installability and CI expectations if not intentional.Overview
Prevents the “domain has location permission” message from showing on DuckDuckGo-owned pages by updating
BrowserTabViewModel.shouldShowLocationPermissionMessageto skip DuckDuckGo and DuckChat URLs, with new unit tests covering those cases.Extends browser UI command handling to support contextual DuckAI updates and adds an external-app prompt option to close the current tab after launching.
Updates debug build packaging/icons in
app/build.gradle(switchesapplicationIdtocom.duckduckgo.dev.android, removes.debugsuffix, and changes debug launcher icons), which may be unintended for a production PR.Written by Cursor Bugbot for commit 28dfbef. This will update automatically on new commits. Configure here.