Skip to content
GitLab
Commit d52a3bf5 authored by Sebastian Kaspari's avatar Sebastian Kaspari
Browse files

Closes #3526: Update selected tab id after selected tab is removed.

parent baf0ee01
Hide whitespace changes
Inline Side-by-side
components/browser/session/src/test/java/mozilla/components/browser/session/SessionManagerMigrationTest.kt
View file @ d52a3bf5
......@@ -10,6 +10,7 @@ import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.support.test.mock
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Test
......@@ -77,4 +78,133 @@ class SessionManagerMigrationTest {
assertEquals("https://www.firefox.com", sessionManager.selectedSessionOrThrow.url)
assertEquals("https://www.firefox.com", selectedTab.content.url)
}
@Test
fun `Remove session and update selection`() {
val state = BrowserState()
val store = BrowserStore(state)
val manager = SessionManager(engine = mock(), store = store)
val session1 = Session(id = "tab1", initialUrl = "https://www.mozilla.org")
val session2 = Session(id = "tab2", initialUrl = "https://www.firefox.com")
val session3 = Session(id = "tab3", initialUrl = "https://wiki.mozilla.org")
val session4 = Session(id = "tab4", initialUrl = "https://github.com/mozilla-mobile/android-components")
manager.add(session1)
manager.add(session2)
manager.add(session3)
manager.add(session4)
// (1), 2, 3, 4
assertEquals(session1, manager.selectedSession)
assertEquals("tab1", store.state.selectedTabId)
// 1, 2, 3, (4)
manager.select(session4)
assertEquals(session4, manager.selectedSession)
assertEquals("tab4", store.state.selectedTabId)
// 1, 2, (3)
manager.remove(session4)
assertEquals(session3, manager.selectedSession)
assertEquals("tab3", store.state.selectedTabId)
// 2, (3)
manager.remove(session1)
assertEquals(session3, manager.selectedSession)
assertEquals("tab3", store.state.selectedTabId)
// (2), 3
manager.select(session2)
assertEquals(session2, manager.selectedSession)
assertEquals("tab2", store.state.selectedTabId)
// (2)
manager.remove(session3)
assertEquals(session2, manager.selectedSession)
assertEquals("tab2", store.state.selectedTabId)
// -
manager.remove(session2)
assertEquals(0, manager.size)
assertNull(store.state.selectedTabId)
}
@Test
fun `Remove private session and select nearby session`() {
val state = BrowserState()
val store = BrowserStore(state)
val manager = SessionManager(engine = mock(), store = store)
assertNull(manager.selectedSession)
val private1 = Session(id = "private1", initialUrl = "https://example.org/private1", private = true)
manager.add(private1)
val regular1 = Session(id = "regular1", initialUrl = "https://www.mozilla.org", private = false)
manager.add(regular1)
val regular2 = Session(id = "regular2", initialUrl = "https://www.firefox.com", private = false)
manager.add(regular2)
val private2 = Session(id = "private2", initialUrl = "https://example.org/private2", private = true)
manager.add(private2)
val private3 = Session(id = "private3", initialUrl = "https://example.org/private3", private = true)
manager.add(private3)
manager.select(private2)
manager.remove(private2)
assertEquals(private3, manager.selectedSession)
assertEquals("private3", store.state.selectedTabId)
manager.remove(private3)
assertEquals(private1, manager.selectedSession)
assertEquals("private1", store.state.selectedTabId)
// Removing the last private session should cause a regular session to be selected
manager.remove(private1)
assertEquals(regular2, manager.selectedSession)
assertEquals("regular2", store.state.selectedTabId)
}
@Test
fun `Remove normal session and select nearby session`() {
val state = BrowserState()
val store = BrowserStore(state)
val manager = SessionManager(engine = mock(), store = store)
assertNull(manager.selectedSession)
assertNull(store.state.selectedTabId)
val regular1 = Session(id = "regular1", initialUrl = "https://www.mozilla.org", private = false)
manager.add(regular1)
val private1 = Session(id = "private1", initialUrl = "https://example.org/private1", private = true)
manager.add(private1)
val private2 = Session(id = "private2", initialUrl = "https://example.org/private2", private = true)
manager.add(private2)
val regular2 = Session(id = "regular2", initialUrl = "https://www.firefox.com", private = false)
manager.add(regular2)
val regular3 = Session(id = "regular3", initialUrl = "https://www.firefox.org", private = false)
manager.add(regular3)
manager.select(regular2)
manager.remove(regular2)
assertEquals(regular3, manager.selectedSession)
assertEquals("regular3", store.state.selectedTabId)
manager.remove(regular3)
assertEquals(regular1, manager.selectedSession)
assertEquals("regular1", store.state.selectedTabId)
// Removing the last regular session should NOT cause a private session to be selected
manager.remove(regular1)
assertNull(manager.selectedSession)
assertNull(store.state.selectedTabId)
}
}
components/browser/state/src/main/java/mozilla/components/browser/state/reducer/TabListReducer.kt
View file @ d52a3bf5
......@@ -8,6 +8,7 @@ import mozilla.components.browser.state.action.TabListAction
import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.TabSessionState
import kotlin.math.max
internal object TabListReducer {
/**
......@@ -26,16 +27,92 @@ internal object TabListReducer {
)
}
is TabListAction.SelectTabAction -> state.copy(selectedTabId = action.tabId)
is TabListAction.SelectTabAction -> {
state.copy(selectedTabId = action.tabId)
}
is TabListAction.RemoveTabAction -> {
val tab = state.findTab(action.tabId)
if (tab == null) {
state
} else {
state.copy(tabs = state.tabs - tab)
val updatedTabList = state.tabs - tab
val updatedSelection = if (state.selectedTabId == tab.id) {
val previousIndex = state.tabs.indexOf(tab)
findNewSelectedTabId(updatedTabList, tab.content.private, previousIndex)
} else {
state.selectedTabId
}
state.copy(
tabs = updatedTabList,
selectedTabId = updatedSelection
)
}
}
}
}
}
/**
* Find a new selected tab and return its id after the tab at [previousIndex] was removed.
*/
private fun findNewSelectedTabId(
tabs: List<TabSessionState>,
isPrivate: Boolean,
previousIndex: Int
): String? {
if (tabs.isEmpty()) {
// There's no tab left to select.
return null
}
val predicate: (TabSessionState) -> Boolean = { tab -> tab.content.private == isPrivate }
// If the previous index is still a valid index and if this is a private/normal tab we are looking for then
// let's use the tab at the same index.
if (previousIndex <= tabs.lastIndex && predicate(tabs[previousIndex])) {
return tabs[previousIndex].id
}
// Find a tab that matches the predicate and is nearby the tab that was selected previously
val nearbyTab = findNearbyTab(tabs, previousIndex, predicate)
return when {
// We found a nearby tab, let's select it.
nearbyTab != null -> nearbyTab.id
// If there's no private tab to select anymore then just select the last regular tab
isPrivate -> tabs.last().id
// Removing the last normal tab should NOT cause a private tab to be selected
else -> null
}
}
/**
* Find a tab that is near the provided [index] and matches the [predicate].
*/
private fun findNearbyTab(
tabs: List<TabSessionState>,
index: Int,
predicate: (TabSessionState) -> Boolean
): TabSessionState? {
val maxSteps = max(tabs.lastIndex - index, index)
if (maxSteps < 0) {
return null
}
// Try tabs oscillating near the index.
for (steps in 1..maxSteps) {
listOf(index - steps, index + steps).forEach { current ->
if (current in 0..tabs.lastIndex &&
predicate(tabs[current])) {
return tabs[current]
}
}
}
return null
}
components/browser/state/src/test/java/mozilla/components/browser/state/action/TabListActionTest.kt
View file @ d52a3bf5
......@@ -4,8 +4,8 @@
package mozilla.components.browser.state.action
import kotlinx.coroutines.runBlocking
import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.createCustomTab
import mozilla.components.browser.state.state.createTab
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.support.test.ext.joinBlocking
......@@ -15,7 +15,7 @@ import org.junit.Test
class TabListActionTest {
@Test
fun `AddTabAction - Adds provided SessionState`() = runBlocking {
fun `AddTabAction - Adds provided SessionState`() {
val state = BrowserState()
val store = BrowserStore(state)
......@@ -25,7 +25,7 @@ class TabListActionTest {
val tab = createTab(url = "https://www.mozilla.org")
store.dispatch(TabListAction.AddTabAction(tab))
.join()
.joinBlocking()
assertEquals(1, store.state.tabs.size)
assertEquals(tab.id, store.state.selectedTabId)
......@@ -71,7 +71,7 @@ class TabListActionTest {
}
@Test
fun `SelectTabAction - Selects SessionState by id`() = runBlocking {
fun `SelectTabAction - Selects SessionState by id`() {
val state = BrowserState(
tabs = listOf(
createTab(id = "a", url = "https://www.mozilla.org"),
......@@ -83,13 +83,13 @@ class TabListActionTest {
assertNull(store.state.selectedTabId)
store.dispatch(TabListAction.SelectTabAction("a"))
.join()
.joinBlocking()
assertEquals("a", store.state.selectedTabId)
}
@Test
fun `RemoveTabAction - Removes SessionState`() = runBlocking {
fun `RemoveTabAction - Removes SessionState`() {
val state = BrowserState(
tabs = listOf(
createTab(id = "a", url = "https://www.mozilla.org"),
......@@ -99,14 +99,14 @@ class TabListActionTest {
val store = BrowserStore(state)
store.dispatch(TabListAction.RemoveTabAction("a"))
.join()
.joinBlocking()
assertEquals(1, store.state.tabs.size)
assertEquals("https://www.firefox.com", store.state.tabs[0].content.url)
}
@Test
fun `RemoveTabAction - Noop for unknown id`() = runBlocking {
fun `RemoveTabAction - Noop for unknown id`() {
val state = BrowserState(
tabs = listOf(
createTab(id = "a", url = "https://www.mozilla.org"),
......@@ -116,10 +116,147 @@ class TabListActionTest {
val store = BrowserStore(state)
store.dispatch(TabListAction.RemoveTabAction("c"))
.join()
.joinBlocking()
assertEquals(2, store.state.tabs.size)
assertEquals("https://www.mozilla.org", store.state.tabs[0].content.url)
assertEquals("https://www.firefox.com", store.state.tabs[1].content.url)
}
}
\ No newline at end of file
@Test
fun `RemoveTabAction - Selected tab id is set to null if selected and last tab is removed`() {
val state = BrowserState(
tabs = listOf(
createTab(id = "a", url = "https://www.mozilla.org")
),
selectedTabId = "a"
)
val store = BrowserStore(state)
assertEquals("a", store.state.selectedTabId)
store.dispatch(TabListAction.RemoveTabAction("a")).joinBlocking()
assertNull(store.state.selectedTabId)
}
@Test
fun `RemoveTabAction - Does not select custom tab`() {
val state = BrowserState(
tabs = listOf(
createTab(id = "a", url = "https://www.mozilla.org")
),
customTabs = listOf(
createCustomTab(id = "b", url = "https://www.firefox.com")
),
selectedTabId = "a"
)
val store = BrowserStore(state)
assertEquals("a", store.state.selectedTabId)
store.dispatch(TabListAction.RemoveTabAction("a")).joinBlocking()
assertNull(store.state.selectedTabId)
}
@Test
fun `RemoveTabAction - Will select next nearby tab after removing selected tab`() {
val state = BrowserState(
tabs = listOf(
createTab(id = "a", url = "https://www.mozilla.org"),
createTab(id = "b", url = "https://www.firefox.com"),
createTab(id = "c", url = "https://www.example.org"),
createTab(id = "d", url = "https://getpocket.com")
),
customTabs = listOf(
createCustomTab(id = "a1", url = "https://www.firefox.com")
),
selectedTabId = "c"
)
val store = BrowserStore(state)
assertEquals("c", store.state.selectedTabId)
store.dispatch(TabListAction.RemoveTabAction("c")).joinBlocking()
assertEquals("d", store.state.selectedTabId)
store.dispatch(TabListAction.RemoveTabAction("a")).joinBlocking()
assertEquals("d", store.state.selectedTabId)
store.dispatch(TabListAction.RemoveTabAction("d")).joinBlocking()
assertEquals("b", store.state.selectedTabId)
store.dispatch(TabListAction.RemoveTabAction("b")).joinBlocking()
assertNull(store.state.selectedTabId)
}
@Test
fun `RemoveTabAction - Selects private tab after private tab was removed`() {
val state = BrowserState(
tabs = listOf(
createTab(id = "a", url = "https://www.mozilla.org", private = true),
createTab(id = "b", url = "https://www.firefox.com", private = false),
createTab(id = "c", url = "https://www.example.org", private = false),
createTab(id = "d", url = "https://getpocket.com", private = true),
createTab(id = "e", url = "https://developer.mozilla.org/", private = true)
),
customTabs = listOf(
createCustomTab(id = "a1", url = "https://www.firefox.com"),
createCustomTab(id = "b1", url = "https://hubs.mozilla.com")
),
selectedTabId = "d"
)
val store = BrowserStore(state)
// [a*, b, c, (d*), e*] -> [a*, b, c, (e*)]
store.dispatch(TabListAction.RemoveTabAction("d")).joinBlocking()
assertEquals("e", store.state.selectedTabId)
// [a*, b, c, (e*)] -> [(a*), b, c]
store.dispatch(TabListAction.RemoveTabAction("e")).joinBlocking()
assertEquals("a", store.state.selectedTabId)
// After removing the last private tab a normal tab will be selected
// [(a*), b, c] -> [b, (c)]
store.dispatch(TabListAction.RemoveTabAction("a")).joinBlocking()
assertEquals("c", store.state.selectedTabId)
}
@Test
fun `RemoveTabAction - Selects normal tab after normal tab was removed`() {
val state = BrowserState(
tabs = listOf(
createTab(id = "a", url = "https://www.mozilla.org", private = false),
createTab(id = "b", url = "https://www.firefox.com", private = true),
createTab(id = "c", url = "https://www.example.org", private = true),
createTab(id = "d", url = "https://getpocket.com", private = false),
createTab(id = "e", url = "https://developer.mozilla.org/", private = false)
),
customTabs = listOf(
createCustomTab(id = "a1", url = "https://www.firefox.com"),
createCustomTab(id = "b1", url = "https://hubs.mozilla.com")
),
selectedTabId = "d"
)
val store = BrowserStore(state)
// [a, b*, c*, (d), e] -> [a, b*, c* (e)]
store.dispatch(TabListAction.RemoveTabAction("d")).joinBlocking()
assertEquals("e", store.state.selectedTabId)
// [a, b*, c*, (e)] -> [(a), b*, c*]
store.dispatch(TabListAction.RemoveTabAction("e")).joinBlocking()
assertEquals("a", store.state.selectedTabId)
// After removing the last normal tab NO private tab should get selected
// [(a), b*, c*] -> [b*, c*]
store.dispatch(TabListAction.RemoveTabAction("a")).joinBlocking()
assertNull(store.state.selectedTabId)
}
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment