Skip to content

Commit ccf3f58

Browse files
authored
Fix: Bookmark loading bug (#6303)
Task/Issue URL: https://app.asana.com/1/137249556945/project/1207418217763355/task/1210647917463143?focus=true ### Description This PR fixes an issue when opening a bookmark from a NTP hides the loaded website. ### Steps to test this PR - [x] Add some bookmark - [x] Make sure the bookmarked site is not open already in another tab, otherwise close it - [x] Open a new NTP - [x] Go to Bookmarks and tap on the saved bookmark - [x] Verify the bookmark is properly loaded in the NTP tab
1 parent 418d972 commit ccf3f58

File tree

8 files changed

+45
-3
lines changed

8 files changed

+45
-3
lines changed

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ import com.duckduckgo.app.browser.model.LongPressTarget
114114
import com.duckduckgo.app.browser.newtab.FavoritesQuickAccessAdapter.QuickAccessFavorite
115115
import com.duckduckgo.app.browser.omnibar.ChangeOmnibarPositionFeature
116116
import com.duckduckgo.app.browser.omnibar.OmnibarEntryConverter
117+
import com.duckduckgo.app.browser.omnibar.QueryOrigin.*
117118
import com.duckduckgo.app.browser.omnibar.model.OmnibarPosition.BOTTOM
118119
import com.duckduckgo.app.browser.omnibar.model.OmnibarPosition.TOP
119120
import com.duckduckgo.app.browser.refreshpixels.RefreshPixelSender
@@ -5569,6 +5570,24 @@ class BrowserTabViewModelTest {
55695570
}
55705571
}
55715572

5573+
@Test
5574+
fun givenRequestSubmittedFromBookmarkThenCorrectQueryOriginSetInBrowserViewState() {
5575+
whenever(mockOmnibarConverter.convertQueryToUrl("foo", null, FromBookmark)).thenReturn("foo.com")
5576+
5577+
testee.onUserSubmittedQuery("foo", queryOrigin = FromBookmark)
5578+
5579+
assertEquals(testee.browserViewState.value?.lastQueryOrigin, FromBookmark)
5580+
}
5581+
5582+
@Test
5583+
fun givenRequestSubmittedFromUserThenCorrectQueryOriginSetInBrowserViewState() {
5584+
whenever(mockOmnibarConverter.convertQueryToUrl("foo", null)).thenReturn("foo.com")
5585+
5586+
testee.onUserSubmittedQuery("foo")
5587+
5588+
assertEquals(testee.browserViewState.value?.lastQueryOrigin, FromUser)
5589+
}
5590+
55725591
@Test
55735592
fun givenSuggestedSearchesDialogShownWhenUserSubmittedQueryThenCustomSearchPixelIsSent() {
55745593
whenever(mockOmnibarConverter.convertQueryToUrl("foo", null)).thenReturn("foo.com")

app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -682,7 +682,7 @@ open class BrowserActivity : DuckDuckGoActivity() {
682682
is Command.LaunchFeedbackView -> startActivity(FeedbackActivity.intent(this))
683683
is Command.SwitchToTab -> openExistingTab(command.tabId)
684684
is Command.OpenInNewTab -> launchNewTab(command.url)
685-
is Command.OpenSavedSite -> currentTab?.submitQuery(command.url)
685+
is Command.OpenSavedSite -> currentTab?.openSavedSite(command.url)
686686
is Command.ShowSetAsDefaultBrowserDialog -> showSetAsDefaultBrowserDialog()
687687
is Command.DismissSetAsDefaultBrowserDialog -> dismissSetAsDefaultBrowserDialog()
688688
is Command.ShowSystemDefaultAppsActivity -> showSystemDefaultAppsActivity(command.intent)

app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ import com.duckduckgo.app.browser.newtab.NewTabPageProvider
148148
import com.duckduckgo.app.browser.omnibar.Omnibar
149149
import com.duckduckgo.app.browser.omnibar.Omnibar.OmnibarTextState
150150
import com.duckduckgo.app.browser.omnibar.Omnibar.ViewMode
151+
import com.duckduckgo.app.browser.omnibar.QueryOrigin
151152
import com.duckduckgo.app.browser.omnibar.experiments.FadeOmnibarItemPressedListener
152153
import com.duckduckgo.app.browser.omnibar.getOmnibarType
153154
import com.duckduckgo.app.browser.omnibar.model.OmnibarPosition.BOTTOM
@@ -1747,6 +1748,10 @@ class BrowserTabFragment :
17471748
}
17481749
}
17491750

1751+
fun openSavedSite(url: String) {
1752+
viewModel.onUserSubmittedQuery(url, QueryOrigin.FromBookmark)
1753+
}
1754+
17501755
fun submitQuery(query: String) {
17511756
viewModel.onUserSubmittedQuery(query)
17521757
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1156,6 +1156,7 @@ class BrowserTabViewModel @Inject constructor(
11561156
sslError = NONE,
11571157
maliciousSiteBlocked = false,
11581158
maliciousSiteStatus = null,
1159+
lastQueryOrigin = queryOrigin,
11591160
)
11601161
autoCompleteViewState.value =
11611162
currentAutoCompleteViewState().copy(showSuggestions = false, showFavorites = false, searchResults = AutoCompleteResult("", emptyList()))
@@ -2873,7 +2874,7 @@ class BrowserTabViewModel @Inject constructor(
28732874
private fun showOrHideKeyboard(cta: Cta?) {
28742875
// we hide the keyboard when showing a DialogCta and HomeCta type in the home screen otherwise we show it
28752876
val shouldHideKeyboard = cta is HomePanelCta || cta is DaxBubbleCta.DaxPrivacyProCta || isBuckExperimentEnabledAndDaxEndCta(cta) ||
2876-
duckChat.showInputScreen.value
2877+
duckChat.showInputScreen.value || currentBrowserViewState().lastQueryOrigin == QueryOrigin.FromBookmark
28772878
command.value = if (shouldHideKeyboard) HideKeyboard else ShowKeyboard
28782879
}
28792880

app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarEntryConverter.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,5 +27,6 @@ interface OmnibarEntryConverter {
2727

2828
sealed class QueryOrigin {
2929
object FromUser : QueryOrigin()
30+
object FromBookmark : QueryOrigin()
3031
data class FromAutocomplete(val isNav: Boolean?) : QueryOrigin()
3132
}

app/src/main/java/com/duckduckgo/app/browser/omnibar/QueryUrlConverter.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ class QueryUrlConverter @Inject constructor(private val requestRewriter: Request
3838
): String {
3939
val isUrl = when (queryOrigin) {
4040
is QueryOrigin.FromAutocomplete -> queryOrigin.isNav
41-
is QueryOrigin.FromUser -> UriString.isWebUrl(searchQuery) || UriString.isDuckUri(searchQuery)
41+
is QueryOrigin.FromUser, QueryOrigin.FromBookmark -> UriString.isWebUrl(searchQuery) || UriString.isDuckUri(searchQuery)
4242
}
4343

4444
if (isUrl == true) {

app/src/main/java/com/duckduckgo/app/browser/viewstate/BrowserViewState.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package com.duckduckgo.app.browser.viewstate
1919
import com.duckduckgo.app.browser.SSLErrorType
2020
import com.duckduckgo.app.browser.SpecialUrlDetector
2121
import com.duckduckgo.app.browser.WebViewErrorResponse
22+
import com.duckduckgo.app.browser.omnibar.QueryOrigin
2223
import com.duckduckgo.app.global.model.MaliciousSiteStatus
2324
import com.duckduckgo.privacyprotectionspopup.api.PrivacyProtectionsPopupViewState
2425
import com.duckduckgo.savedsites.api.models.SavedSite
@@ -58,6 +59,7 @@ data class BrowserViewState(
5859
val maliciousSiteStatus: MaliciousSiteStatus? = null,
5960
val privacyProtectionsPopupViewState: PrivacyProtectionsPopupViewState = PrivacyProtectionsPopupViewState.Gone,
6061
val showDuckChatOption: Boolean = false,
62+
val lastQueryOrigin: QueryOrigin = QueryOrigin.FromUser,
6163
)
6264

6365
sealed class HighlightableButton {

app/src/test/java/com/duckduckgo/app/browser/QueryUrlConverterTest.kt

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,20 @@ class QueryUrlConverterTest {
102102
assertEquals(input, result)
103103
}
104104

105+
@Test
106+
fun whenQueryOriginIsFromBookmarkAndIsQueryThenSearchQueryBuilt() {
107+
val input = "foo"
108+
val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromBookmark)
109+
assertDuckDuckGoSearchQuery("foo", result)
110+
}
111+
112+
@Test
113+
fun whenQueryOriginIsFromBookmarkAndIsUrlThenUrlReturned() {
114+
val input = "http://example.com"
115+
val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromBookmark)
116+
assertEquals(input, result)
117+
}
118+
105119
@Test
106120
fun whenQueryOriginIsFromAutocompleteAndIsNavIsFalseThenSearchQueryBuilt() {
107121
val input = "example.com"

0 commit comments

Comments
 (0)