Skip to content

Commit b11cb7b

Browse files
marcosholgadomarcosholgado
authored
Fix url state when input url is different than loaded url (#4659)
Task/Issue URL: https://app.asana.com/0/414730916066338/1207666379072710/f ### Description Change URL state when input URL is different than URL loaded ### Steps to test this PR - [x] Go to https://weather.com - [x] Change URL to http://weather.com - [x] URL bar should change to https: - [x] Change again to http:// - [x] URL bar should change again to https and show the shield --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1207666379072710
1 parent 1d3ce74 commit b11cb7b

File tree

6 files changed

+96
-11
lines changed

6 files changed

+96
-11
lines changed

‎app/build.gradle‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,8 +437,10 @@ dependencies {
437437
androidTestImplementation "androidx.test.ext:junit-ktx:_"
438438
androidTestImplementation "androidx.test.espresso:espresso-core:_"
439439
androidTestImplementation "androidx.test.espresso:espresso-web:_"
440+
androidTestImplementation project(':feature-toggles-test')
440441

441442
testImplementation project(':vpn-api-test')
443+
testImplementation project(':feature-toggles-test')
442444
testImplementation "org.mockito.kotlin:mockito-kotlin:_"
443445
testImplementation Testing.junit4
444446
testImplementation AndroidX.archCore.testing

‎app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,8 @@ import com.duckduckgo.common.utils.device.DeviceInfo
167167
import com.duckduckgo.downloads.api.DownloadStateListener
168168
import com.duckduckgo.downloads.api.FileDownloader
169169
import com.duckduckgo.downloads.api.FileDownloader.PendingFileDownload
170+
import com.duckduckgo.feature.toggles.api.FakeFeatureToggleFactory
171+
import com.duckduckgo.feature.toggles.api.FakeToggleStore
170172
import com.duckduckgo.feature.toggles.api.FeatureToggle
171173
import com.duckduckgo.feature.toggles.api.Toggle
172174
import com.duckduckgo.history.api.HistoryEntry.VisitedPage
@@ -472,6 +474,7 @@ class BrowserTabViewModelTest {
472474
private val mockUserBrowserProperties: UserBrowserProperties = mock()
473475
private val mockAutoCompleteRepository: AutoCompleteRepository = mock()
474476
private val commandActionMapper: CommandActionMapper = mock()
477+
private val newStateKillSwitch = FakeFeatureToggleFactory.create(NewStateKillSwitch::class.java, FakeToggleStore())
475478

476479
@Before
477480
fun before() {
@@ -554,7 +557,7 @@ class BrowserTabViewModelTest {
554557
whenever(fireproofDialogsEventHandler.event).thenReturn(fireproofDialogsEventHandlerLiveData)
555558
whenever(cameraHardwareChecker.hasCameraHardware()).thenReturn(true)
556559
whenever(mockPrivacyProtectionsPopupManager.viewState).thenReturn(flowOf(PrivacyProtectionsPopupViewState.Gone))
557-
560+
newStateKillSwitch.self().setEnabled(Toggle.State(true))
558561
testee = BrowserTabViewModel(
559562
statisticsUpdater = mockStatisticsUpdater,
560563
queryUrlConverter = mockOmnibarConverter,
@@ -619,6 +622,7 @@ class BrowserTabViewModelTest {
619622
userBrowserProperties = mockUserBrowserProperties,
620623
history = mockNavigationHistory,
621624
commandActionMapper = commandActionMapper,
625+
newStateKillSwitch = newStateKillSwitch,
622626
)
623627

624628
testee.loadData("abc", null, false)
@@ -5345,6 +5349,39 @@ class BrowserTabViewModelTest {
53455349
verify(mockPixel).fire(OnboardingExperimentPixel.PixelName.ONBOARDING_VISIT_SITE_CUSTOM)
53465350
}
53475351

5352+
@Test
5353+
fun whenNavigationStateUnchangedButDifferentUrlThenUpdateUrl() {
5354+
val httpUrl = "http://example.com/"
5355+
val httpsUrl = "https://example.com/"
5356+
loadUrl(httpsUrl, isBrowserShowing = true)
5357+
assertEquals(httpsUrl, testee.url)
5358+
assertEquals(httpsUrl, omnibarViewState().omnibarText)
5359+
testee.siteLiveData.value?.url = httpUrl
5360+
assertEquals(httpUrl, testee.siteLiveData.value?.url)
5361+
5362+
updateUrl(httpsUrl, httpsUrl, true)
5363+
assertEquals(httpsUrl, testee.url)
5364+
assertEquals(httpsUrl, testee.siteLiveData.value?.url)
5365+
assertEquals(httpsUrl, omnibarViewState().omnibarText)
5366+
}
5367+
5368+
@Test
5369+
fun whenNavigationStateUnchangedButDifferentUrlAndFeatureDisableThenDoNotUpdateUrl() {
5370+
newStateKillSwitch.self().setEnabled(Toggle.State(false))
5371+
val httpUrl = "http://example.com/"
5372+
val httpsUrl = "https://example.com/"
5373+
loadUrl(httpsUrl, isBrowserShowing = true)
5374+
assertEquals(httpsUrl, testee.url)
5375+
assertEquals(httpsUrl, omnibarViewState().omnibarText)
5376+
testee.siteLiveData.value?.url = httpUrl
5377+
assertEquals(httpUrl, testee.siteLiveData.value?.url)
5378+
5379+
updateUrl(httpsUrl, httpsUrl, true)
5380+
assertEquals(httpUrl, testee.url)
5381+
assertEquals(httpUrl, testee.siteLiveData.value?.url)
5382+
assertEquals(httpsUrl, omnibarViewState().omnibarText)
5383+
}
5384+
53485385
private fun aCredential(): LoginCredentials {
53495386
return LoginCredentials(domain = null, username = null, password = null)
53505387
}

‎app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,7 @@ class BrowserTabViewModel @Inject constructor(
269269
private val userBrowserProperties: UserBrowserProperties,
270270
private val history: NavigationHistory,
271271
private val commandActionMapper: CommandActionMapper,
272+
private val newStateKillSwitch: NewStateKillSwitch,
272273
) : WebViewClientListener,
273274
EditSavedSiteListener,
274275
DeleteBookmarkListener,
@@ -1093,7 +1094,16 @@ class BrowserTabViewModel @Inject constructor(
10931094
is WebNavigationStateChange.PageCleared -> pageCleared()
10941095
is WebNavigationStateChange.UrlUpdated -> urlUpdated(stateChange.url)
10951096
is WebNavigationStateChange.PageNavigationCleared -> disableUserNavigation()
1096-
else -> {}
1097+
is WebNavigationStateChange.Unchanged -> {
1098+
stateChange.url?.let {
1099+
updateWhenInputUrlIsDifferentThanLoadedUrl(it)
1100+
}
1101+
}
1102+
is WebNavigationStateChange.Other -> {
1103+
stateChange.url?.let {
1104+
updateWhenInputUrlIsDifferentThanLoadedUrl(it)
1105+
}
1106+
}
10971107
}
10981108

10991109
if ((newWebNavigationState.progress ?: 0) >= SHOW_CONTENT_MIN_PROGRESS) {
@@ -1102,6 +1112,12 @@ class BrowserTabViewModel @Inject constructor(
11021112
navigationAwareLoginDetector.onEvent(NavigationEvent.WebNavigationEvent(stateChange))
11031113
}
11041114

1115+
private fun updateWhenInputUrlIsDifferentThanLoadedUrl(loadedUrl: String) {
1116+
if (newStateKillSwitch.self().isEnabled() && loadedUrl != site?.url) {
1117+
urlUpdated(loadedUrl)
1118+
}
1119+
}
1120+
11051121
override fun onPageContentStart(url: String) {
11061122
showWebContent()
11071123
}

‎app/src/main/java/com/duckduckgo/app/browser/NewStateKillSwitch.kt‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*
2+
* Copyright (c) 2024 DuckDuckGo
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.duckduckgo.app.browser
18+
19+
import com.duckduckgo.anvil.annotations.ContributesRemoteFeature
20+
import com.duckduckgo.di.scopes.AppScope
21+
import com.duckduckgo.feature.toggles.api.Toggle
22+
23+
@ContributesRemoteFeature(
24+
scope = AppScope::class,
25+
featureName = "androidNewStateKillSwitch",
26+
)
27+
interface NewStateKillSwitch {
28+
@Toggle.DefaultValue(true)
29+
fun self(): Toggle
30+
}

‎app/src/main/java/com/duckduckgo/app/browser/WebNavigationState.kt‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,14 @@ sealed class WebNavigationStateChange {
4141

4242
data class UrlUpdated(val url: String) : WebNavigationStateChange()
4343
object PageCleared : WebNavigationStateChange()
44-
object Unchanged : WebNavigationStateChange()
44+
data class Unchanged(val url: String?) : WebNavigationStateChange()
4545
object PageNavigationCleared : WebNavigationStateChange()
46-
object Other : WebNavigationStateChange()
46+
data class Other(val url: String?) : WebNavigationStateChange()
4747
}
4848

4949
fun WebNavigationState.compare(previous: WebNavigationState?): WebNavigationStateChange {
5050
if (this == previous) {
51-
return WebNavigationStateChange.Unchanged
51+
return WebNavigationStateChange.Unchanged(currentUrl)
5252
}
5353

5454
if (this is EmptyNavigationState) {
@@ -59,7 +59,7 @@ fun WebNavigationState.compare(previous: WebNavigationState?): WebNavigationStat
5959
return WebNavigationStateChange.PageCleared
6060
}
6161

62-
val latestUrl = currentUrl ?: return WebNavigationStateChange.Other
62+
val latestUrl = currentUrl ?: return WebNavigationStateChange.Other(null)
6363

6464
// A new page load is identified by the original url changing
6565
if (originalUrl != previous?.originalUrl) {
@@ -76,7 +76,7 @@ fun WebNavigationState.compare(previous: WebNavigationState?): WebNavigationStat
7676
return WebNavigationStateChange.UrlUpdated(latestUrl)
7777
}
7878

79-
return WebNavigationStateChange.Other
79+
return WebNavigationStateChange.Other(currentUrl)
8080
}
8181

8282
data class WebViewNavigationState(

‎app/src/test/java/com/duckduckgo/app/browser/WebNavigationStateComparisonTest.kt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,14 @@ class WebNavigationStateComparisonTest {
3333
@Test
3434
fun whenPreviousStateAndLatestStateSameThenCompareReturnsUnchanged() {
3535
val state = buildState("http://foo.com", "http://subdomain.foo.com")
36-
assertEquals(Unchanged, state.compare(state))
36+
assertEquals(Unchanged("http://subdomain.foo.com"), state.compare(state))
3737
}
3838

3939
@Test
4040
fun whenPreviousStateAndLatestStateEqualThenCompareReturnsUnchanged() {
4141
val previousState = buildState("http://foo.com", "http://subdomain.foo.com")
4242
val latestState = buildState("http://foo.com", "http://subdomain.foo.com")
43-
assertEquals(Unchanged, latestState.compare(previousState))
43+
assertEquals(Unchanged("http://subdomain.foo.com"), latestState.compare(previousState))
4444
}
4545

4646
@Test
@@ -124,14 +124,14 @@ class WebNavigationStateComparisonTest {
124124
fun whenPreviousContainsAnOriginalUrlAndCurrentUrlAndLatestContainsSameOriginalUrlAndNoCurrentUrlThenCompareReturnsOther() {
125125
val previousState = buildState("http://same.com", "http://subdomain.previous.com")
126126
val latestState = buildState("http://same.com", null)
127-
assertEquals(Other, latestState.compare(previousState))
127+
assertEquals(Other(null), latestState.compare(previousState))
128128
}
129129

130130
@Test
131131
fun whenPreviousContainsAnOriginalUrlAndCurrentUrlAndLatestStateContainsDifferentOriginalUrlAndNoCurrentUrlThenCompareReturnsOther() {
132132
val previousState = buildState("http://previous.com", "http://subdomain.previous.com")
133133
val latestState = buildState("http://latest.com", null)
134-
assertEquals(Other, latestState.compare(previousState))
134+
assertEquals(Other(null), latestState.compare(previousState))
135135
}
136136

137137
private fun buildState(

0 commit comments

Comments
 (0)