[fenix] Issue https://github.com/mozilla-mobile/fenix/issues/21707: Correct search grouping logic in extensions

At this moment, we have two extension methods that have duplicate
functionality to construct search term groupings. One on `List<Tab>` and
one on `List<TabSessionState>`. The former is used for everything
related to tabs piped through the `TabsFeature` and the latter is for
consumers of `BrowserState` directly.

The bug occurs because our implementation of search groupings was
updated only on the former extension, but the `HeaderBinding`, that
observes the BrowserState and updates the title visibility, was using
the latter.

Ideally, we remove this duplication when we no longer have separate data
classes for consumers of `TabsFeature`, but this intermediary fix should
suffice.
pull/600/head
Jonathan Almeida 3 years ago committed by mergify[bot]
parent cb7cc7a7eb
commit 18a8788812

@ -60,7 +60,7 @@ val BrowserState.inProgressMediaTab: TabSessionState?
*/
val BrowserState.lastSearchGroup: RecentTab.SearchGroup?
get() {
val tabGroup = normalTabs.toSearchGroup().lastOrNull { it.tabs.count() > 1 } ?: return null
val tabGroup = normalTabs.toSearchGroup().first.lastOrNull() ?: return null
val firstTab = tabGroup.tabs.firstOrNull() ?: return null
return RecentTab.SearchGroup(
@ -73,9 +73,10 @@ val BrowserState.lastSearchGroup: RecentTab.SearchGroup?
}
/**
* Get search term groups sorted by last access time.
* Returns a pair containing a list of search term groups sorted by last access time, and "remainder" tabs that have
* search terms but should not be in groups (because the group is of size one).
*/
fun List<TabSessionState>.toSearchGroup(): List<TabGroup> {
fun List<TabSessionState>.toSearchGroup(): Pair<List<TabGroup>, List<TabSessionState>> {
val data = filter {
it.isNormalTabActiveWithSearchTerm(maxActiveTime)
}.groupBy {
@ -85,7 +86,7 @@ fun List<TabSessionState>.toSearchGroup(): List<TabGroup> {
}.lowercase()
}
return data.map { mapEntry ->
val groupings = data.map { mapEntry ->
val searchTerm = mapEntry.key.replaceFirstChar(Char::uppercase)
val groupTabs = mapEntry.value
val groupMax = groupTabs.fold(0L) { acc, tab ->
@ -97,5 +98,10 @@ fun List<TabSessionState>.toSearchGroup(): List<TabGroup> {
tabs = groupTabs,
lastAccess = groupMax
)
}.sortedBy { it.lastAccess }
}
val groups = groupings.filter { it.tabs.size > 1 }.sortedBy { it.lastAccess }
val remainderTabs = (groupings - groups).flatMap { it.tabs }
return groups to remainderTabs
}

@ -48,11 +48,13 @@ fun BrowserState.getNormalTrayTabs(
): List<TabSessionState> {
return normalTabs.run {
if (searchTermTabGroupsAreEnabled && inactiveTabsEnabled) {
filter { it.isNormalTabActiveWithoutSearchTerm(maxActiveTime) }
val remainderTabs = toSearchGroup().second
filter { it.isNormalTabActiveWithoutSearchTerm(maxActiveTime) } + remainderTabs
} else if (inactiveTabsEnabled) {
filter { it.isNormalTabActive(maxActiveTime) }
} else if (searchTermTabGroupsAreEnabled) {
filter { it.isNormalTabWithSearchTerm() }
val remainderTabs = toSearchGroup().second
filter { it.isNormalTabWithSearchTerm() } + remainderTabs
} else {
this
}
@ -65,7 +67,7 @@ fun BrowserState.getNormalTrayTabs(
fun BrowserState.getSearchTabGroups(
searchTermTabGroupsAreEnabled: Boolean
): List<TabGroup> = if (searchTermTabGroupsAreEnabled) {
normalTabs.toSearchGroup().filter { it.tabs.size > 1 }
normalTabs.toSearchGroup().first
} else {
emptyList()
}

Loading…
Cancel
Save