For #1824: Prevent extremely long URLs from locking up HomeFragment
parent
fed3f2fe91
commit
4e25f41a41
|
@ -11,6 +11,7 @@ import android.view.ViewOutlineProvider
|
||||||
import androidx.appcompat.content.res.AppCompatResources
|
import androidx.appcompat.content.res.AppCompatResources
|
||||||
import kotlinx.android.synthetic.main.tab_list_row.*
|
import kotlinx.android.synthetic.main.tab_list_row.*
|
||||||
import mozilla.components.browser.state.state.MediaState
|
import mozilla.components.browser.state.state.MediaState
|
||||||
|
import mozilla.components.browser.toolbar.MAX_URI_LENGTH
|
||||||
import mozilla.components.support.ktx.android.util.dpToFloat
|
import mozilla.components.support.ktx.android.util.dpToFloat
|
||||||
import org.mozilla.fenix.R
|
import org.mozilla.fenix.R
|
||||||
import org.mozilla.fenix.components.metrics.Event
|
import org.mozilla.fenix.components.metrics.Event
|
||||||
|
@ -81,7 +82,12 @@ class TabViewHolder(
|
||||||
internal fun bindSession(tab: Tab) {
|
internal fun bindSession(tab: Tab) {
|
||||||
updateTab(tab)
|
updateTab(tab)
|
||||||
updateTitle(tab.title)
|
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)
|
updateFavIcon(tab.url, tab.icon)
|
||||||
updateSelected(tab.selected ?: false)
|
updateSelected(tab.selected ?: false)
|
||||||
updatePlayPauseButton(tab.mediaState)
|
updatePlayPauseButton(tab.mediaState)
|
||||||
|
|
|
@ -8,11 +8,13 @@ import android.view.View
|
||||||
import android.widget.ImageButton
|
import android.widget.ImageButton
|
||||||
import android.widget.ImageView
|
import android.widget.ImageView
|
||||||
import android.widget.TextView
|
import android.widget.TextView
|
||||||
|
import androidx.annotation.VisibleForTesting
|
||||||
import androidx.appcompat.widget.AppCompatImageButton
|
import androidx.appcompat.widget.AppCompatImageButton
|
||||||
import androidx.core.content.ContextCompat
|
import androidx.core.content.ContextCompat
|
||||||
import mozilla.components.browser.state.state.MediaState
|
import mozilla.components.browser.state.state.MediaState
|
||||||
import mozilla.components.browser.tabstray.TabViewHolder
|
import mozilla.components.browser.tabstray.TabViewHolder
|
||||||
import mozilla.components.browser.tabstray.thumbnail.TabThumbnailView
|
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.Tab
|
||||||
import mozilla.components.concept.tabstray.TabsTray
|
import mozilla.components.concept.tabstray.TabsTray
|
||||||
import mozilla.components.feature.media.ext.pauseIfPlaying
|
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 titleView: TextView = itemView.findViewById(R.id.mozac_browser_tabstray_title)
|
||||||
private val closeView: AppCompatImageButton = itemView.findViewById(R.id.mozac_browser_tabstray_close)
|
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 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)
|
private val playPauseButtonView: ImageButton = itemView.findViewById(R.id.play_pause_button)
|
||||||
|
|
||||||
override var tab: Tab? = null
|
override var tab: Tab? = null
|
||||||
|
@ -132,10 +135,16 @@ class TabTrayViewHolder(itemView: View) : TabViewHolder(itemView) {
|
||||||
titleView.text = title
|
titleView.text = title
|
||||||
}
|
}
|
||||||
private fun updateUrl(tab: Tab) {
|
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) {
|
val itemBackground = if (isSelected) {
|
||||||
R.attr.tabTrayItemSelectedBackground
|
R.attr.tabTrayItemSelectedBackground
|
||||||
} else {
|
} else {
|
||||||
|
|
|
@ -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)
|
||||||
|
}
|
||||||
|
}
|
|
@ -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)
|
||||||
|
}
|
||||||
|
}
|
Loading…
Reference in New Issue