From a8c53c6bf1bfd5fb5aaa0c3961f11643a3266406 Mon Sep 17 00:00:00 2001 From: Roger Yang Date: Tue, 23 Mar 2021 10:31:53 -0400 Subject: [PATCH] Closes #17791: Use updated URL with custom tabs when copying to clipboard (#18590) --- .../components/toolbar/BrowserToolbarView.kt | 2 +- .../mozilla/fenix/utils/ToolbarPopupWindow.kt | 15 ++++--- .../fenix/utils/ToolbarPopupWindowTest.kt | 45 +++++++++++++++---- 3 files changed, 45 insertions(+), 17 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt index b3b97311d..b64100299 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt @@ -87,7 +87,7 @@ class BrowserToolbarView( view.display.setOnUrlLongClickListener { ToolbarPopupWindow.show( WeakReference(view), - customTabSession, + customTabSession?.id, interactor::onBrowserToolbarPasteAndGo, interactor::onBrowserToolbarPaste ) diff --git a/app/src/main/java/org/mozilla/fenix/utils/ToolbarPopupWindow.kt b/app/src/main/java/org/mozilla/fenix/utils/ToolbarPopupWindow.kt index f9b349fce..db2d0b5a9 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/ToolbarPopupWindow.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/ToolbarPopupWindow.kt @@ -16,18 +16,18 @@ import androidx.core.view.isVisible import com.google.android.material.snackbar.Snackbar import kotlinx.android.synthetic.main.browser_toolbar_popup_window.view.* import mozilla.components.browser.state.selector.selectedTab -import mozilla.components.browser.state.state.CustomTabSessionState import mozilla.components.browser.state.store.BrowserStore import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components import java.lang.ref.WeakReference +import mozilla.components.browser.state.selector.findCustomTab object ToolbarPopupWindow { fun show( view: WeakReference, - customTabSession: CustomTabSessionState? = null, + customTabId: String? = null, handlePasteAndGo: (String) -> Unit, handlePaste: (String) -> Unit, copyVisible: Boolean = true @@ -36,7 +36,7 @@ object ToolbarPopupWindow { val clipboard = context.components.clipboardHandler if (!copyVisible && clipboard.text.isNullOrEmpty()) return - val isCustomTabSession = customTabSession != null + val isCustomTabSession = customTabId != null val customView = LayoutInflater.from(context) .inflate(R.layout.browser_toolbar_popup_window, null) @@ -63,7 +63,7 @@ object ToolbarPopupWindow { popupWindow.dismiss() clipboard.text = getUrlForClipboard( it.context.components.core.store, - customTabSession + customTabId ) view.get()?.let { @@ -101,10 +101,11 @@ object ToolbarPopupWindow { @VisibleForTesting internal fun getUrlForClipboard( store: BrowserStore, - customTabSession: CustomTabSessionState? = null + customTabId: String? = null ): String? { - return if (customTabSession != null) { - customTabSession.content.url + return if (customTabId != null) { + val customTab = store.state.findCustomTab(customTabId) + customTab?.content?.url } else { val selectedTab = store.state.selectedTab selectedTab?.readerState?.activeUrl ?: selectedTab?.content?.url diff --git a/app/src/test/java/org/mozilla/fenix/utils/ToolbarPopupWindowTest.kt b/app/src/test/java/org/mozilla/fenix/utils/ToolbarPopupWindowTest.kt index 69ef82b3a..d56f1b50b 100644 --- a/app/src/test/java/org/mozilla/fenix/utils/ToolbarPopupWindowTest.kt +++ b/app/src/test/java/org/mozilla/fenix/utils/ToolbarPopupWindowTest.kt @@ -4,35 +4,44 @@ package org.mozilla.fenix.utils -import io.mockk.mockk +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestCoroutineDispatcher +import mozilla.components.browser.state.action.ContentAction import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.ReaderState 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 +import mozilla.components.support.test.rule.MainCoroutineRule import org.junit.Assert.assertEquals +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.helpers.FenixRobolectricTestRunner +@ExperimentalCoroutinesApi @RunWith(FenixRobolectricTestRunner::class) class ToolbarPopupWindowTest { + private val testDispatcher = TestCoroutineDispatcher() - @Test - fun getUrlForClipboard() { - val customTabSession = createCustomTab("https://mozilla.org") + @get:Rule + val coroutinesTestRule = MainCoroutineRule(testDispatcher) + @Test + fun `getUrlForClipboard should get the right URL`() { // Custom tab + val customTabSession = createCustomTab("https://mozilla.org") + var store = BrowserStore(BrowserState(customTabs = listOf(customTabSession))) assertEquals( "https://mozilla.org", - ToolbarPopupWindow.getUrlForClipboard(mockk(), customTabSession) + ToolbarPopupWindow.getUrlForClipboard(store, customTabSession.id) ) // Regular tab val regularTab = createTab(url = "http://firefox.com") - var store = - BrowserStore(BrowserState(tabs = listOf(regularTab), selectedTabId = regularTab.id)) - assertEquals(regularTab.content.url, ToolbarPopupWindow.getUrlForClipboard(store)) + store = BrowserStore(BrowserState(tabs = listOf(regularTab), selectedTabId = regularTab.id)) + assertEquals("http://firefox.com", ToolbarPopupWindow.getUrlForClipboard(store)) // Reader Tab val readerTab = createTab( @@ -40,6 +49,24 @@ class ToolbarPopupWindowTest { readerState = ReaderState(active = true, activeUrl = "https://blog.mozilla.org/123") ) store = BrowserStore(BrowserState(tabs = listOf(readerTab), selectedTabId = readerTab.id)) - assertEquals(readerTab.readerState.activeUrl, ToolbarPopupWindow.getUrlForClipboard(store)) + assertEquals("https://blog.mozilla.org/123", ToolbarPopupWindow.getUrlForClipboard(store)) + } + + @Test + fun `getUrlForClipboard should get the updated URL`() { + // Custom tab + val customTabSession = createCustomTab("https://mozilla.org") + var store = BrowserStore(BrowserState(customTabs = listOf(customTabSession))) + store.dispatch(ContentAction.UpdateUrlAction(customTabSession.id, "https://firefox.com")).joinBlocking() + assertEquals( + "https://firefox.com", + ToolbarPopupWindow.getUrlForClipboard(store, customTabSession.id) + ) + + // Regular tab + val regularTab = createTab(url = "http://firefox.com") + store = BrowserStore(BrowserState(tabs = listOf(regularTab), selectedTabId = regularTab.id)) + store.dispatch(ContentAction.UpdateUrlAction(regularTab.id, "https://mozilla.org")).joinBlocking() + assertEquals("https://mozilla.org", ToolbarPopupWindow.getUrlForClipboard(store)) } }