From 4e25f41a41fbff20e92c3f36879ba2a9a79c35e3 Mon Sep 17 00:00:00 2001 From: Christian Sadilek Date: Thu, 21 May 2020 10:49:18 -0400 Subject: [PATCH] For #1824: Prevent extremely long URLs from locking up HomeFragment --- .../viewholders/TabViewHolder.kt | 8 +++- .../fenix/tabtray/TabTrayViewHolder.kt | 15 +++++-- .../mozilla/fenix/home/TabViewHolderTest.kt | 43 +++++++++++++++++++ .../fenix/tabtray/TabTrayViewHolderTest.kt | 39 +++++++++++++++++ 4 files changed, 101 insertions(+), 4 deletions(-) create mode 100644 app/src/test/java/org/mozilla/fenix/home/TabViewHolderTest.kt create mode 100644 app/src/test/java/org/mozilla/fenix/tabtray/TabTrayViewHolderTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabViewHolder.kt index 6cce804fd..776646aeb 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabViewHolder.kt @@ -11,6 +11,7 @@ import android.view.ViewOutlineProvider import androidx.appcompat.content.res.AppCompatResources import kotlinx.android.synthetic.main.tab_list_row.* import mozilla.components.browser.state.state.MediaState +import mozilla.components.browser.toolbar.MAX_URI_LENGTH import mozilla.components.support.ktx.android.util.dpToFloat import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.Event @@ -81,7 +82,12 @@ class TabViewHolder( internal fun bindSession(tab: Tab) { updateTab(tab) updateTitle(tab.title) - updateHostname(tab.hostname) + // Truncate to MAX_URI_LENGTH to prevent the UI from locking up for + // extremely large URLs such as data URIs or bookmarklets. The same + // is done in the toolbar and awesomebar: + // https://github.com/mozilla-mobile/fenix/issues/1824 + // https://github.com/mozilla-mobile/android-components/issues/6985 + updateHostname(tab.hostname.take(MAX_URI_LENGTH)) updateFavIcon(tab.url, tab.icon) updateSelected(tab.selected ?: false) updatePlayPauseButton(tab.mediaState) diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayViewHolder.kt index df4d7fdf0..63ecce51a 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayViewHolder.kt @@ -8,11 +8,13 @@ import android.view.View import android.widget.ImageButton import android.widget.ImageView import android.widget.TextView +import androidx.annotation.VisibleForTesting import androidx.appcompat.widget.AppCompatImageButton import androidx.core.content.ContextCompat import mozilla.components.browser.state.state.MediaState import mozilla.components.browser.tabstray.TabViewHolder import mozilla.components.browser.tabstray.thumbnail.TabThumbnailView +import mozilla.components.browser.toolbar.MAX_URI_LENGTH import mozilla.components.concept.tabstray.Tab import mozilla.components.concept.tabstray.TabsTray import mozilla.components.feature.media.ext.pauseIfPlaying @@ -37,7 +39,8 @@ class TabTrayViewHolder(itemView: View) : TabViewHolder(itemView) { private val titleView: TextView = itemView.findViewById(R.id.mozac_browser_tabstray_title) private val closeView: AppCompatImageButton = itemView.findViewById(R.id.mozac_browser_tabstray_close) private val thumbnailView: TabThumbnailView = itemView.findViewById(R.id.mozac_browser_tabstray_thumbnail) - private val urlView: TextView? = itemView.findViewById(R.id.mozac_browser_tabstray_url) + @VisibleForTesting + internal val urlView: TextView? = itemView.findViewById(R.id.mozac_browser_tabstray_url) private val playPauseButtonView: ImageButton = itemView.findViewById(R.id.play_pause_button) override var tab: Tab? = null @@ -132,10 +135,16 @@ class TabTrayViewHolder(itemView: View) : TabViewHolder(itemView) { titleView.text = title } private fun updateUrl(tab: Tab) { - urlView?.text = tab.url.tryGetHostFromUrl() + // Truncate to MAX_URI_LENGTH to prevent the UI from locking up for + // extremely large URLs such as data URIs or bookmarklets. The same + // is done in the toolbar and awesomebar: + // https://github.com/mozilla-mobile/fenix/issues/1824 + // https://github.com/mozilla-mobile/android-components/issues/6985 + urlView?.text = tab.url.tryGetHostFromUrl().take(MAX_URI_LENGTH) } - private fun updateBackgroundColor(isSelected: Boolean) { + @VisibleForTesting + internal fun updateBackgroundColor(isSelected: Boolean) { val itemBackground = if (isSelected) { R.attr.tabTrayItemSelectedBackground } else { diff --git a/app/src/test/java/org/mozilla/fenix/home/TabViewHolderTest.kt b/app/src/test/java/org/mozilla/fenix/home/TabViewHolderTest.kt new file mode 100644 index 000000000..67f42109e --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/home/TabViewHolderTest.kt @@ -0,0 +1,43 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.home + +import android.view.LayoutInflater +import io.mockk.mockk +import kotlinx.android.synthetic.main.tab_list_row.view.* +import mozilla.components.browser.state.state.MediaState +import mozilla.components.browser.toolbar.MAX_URI_LENGTH +import mozilla.components.support.test.robolectric.testContext +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.R +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner +import org.mozilla.fenix.home.sessioncontrol.TabSessionInteractor +import org.mozilla.fenix.home.sessioncontrol.viewholders.TabViewHolder + +@RunWith(FenixRobolectricTestRunner::class) +class TabViewHolderTest { + + @Test + fun `extremely long URLs are truncated to prevent slowing down the UI`() { + val view = LayoutInflater.from(testContext).inflate( + R.layout.tab_list_row, null, false) + + val interactor: TabSessionInteractor = mockk() + val tabViewHolder = TabViewHolder(view, interactor) + + val extremelyLongUrl = "m".repeat(MAX_URI_LENGTH + 1) + val tab = Tab( + sessionId = "123", + url = extremelyLongUrl, + hostname = extremelyLongUrl, + title = "test", + mediaState = MediaState.State.NONE) + tabViewHolder.bindSession(tab) + + assertEquals("m".repeat(MAX_URI_LENGTH), view.hostname.text) + } +} diff --git a/app/src/test/java/org/mozilla/fenix/tabtray/TabTrayViewHolderTest.kt b/app/src/test/java/org/mozilla/fenix/tabtray/TabTrayViewHolderTest.kt new file mode 100644 index 000000000..a186b914e --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/tabtray/TabTrayViewHolderTest.kt @@ -0,0 +1,39 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.tabtray + +import android.view.LayoutInflater +import androidx.test.core.app.ApplicationProvider +import io.mockk.mockk +import mozilla.components.browser.toolbar.MAX_URI_LENGTH +import mozilla.components.concept.tabstray.Tab +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mockito.doNothing +import org.mockito.Mockito.spy +import org.mozilla.fenix.R +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner + +@RunWith(FenixRobolectricTestRunner::class) +class TabTrayViewHolderTest { + + @Test + fun `extremely long URLs are truncated to prevent slowing down the UI`() { + val view = LayoutInflater.from(ApplicationProvider.getApplicationContext()).inflate( + R.layout.tab_tray_item, null, false) + + val tabViewHolder = spy(TabTrayViewHolder(view)) + doNothing().`when`(tabViewHolder).updateBackgroundColor(false) + + val extremelyLongUrl = "m".repeat(MAX_URI_LENGTH + 1) + val tab = Tab( + id = "123", + url = extremelyLongUrl) + tabViewHolder.bind(tab, false, mockk()) + + assertEquals("m".repeat(MAX_URI_LENGTH), tabViewHolder.urlView?.text) + } +}