Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
Merge #7031
Browse files Browse the repository at this point in the history
7031: Closes #5249: Long URI in the address bar will hang the UI r=pocmo a=csadilek

We've had two cases now where this was a problem. A bookmarklet (see #6985) and a data URI (#5249).

This fix is similar to the one we landed last week for our Awesomebar (csadilek@bba846d), but tapping on long URLs or copying them from the clipboard etc. would still cause the UI to hang. With data URIs I can reproduce seconds/minutes long hangs that are fixed with this.

Took this from Fennec (https://github.com/pocmo/fennec-graveyard/blob/master/mobile/android/chrome/content/browser.js#L4308), and applied to both display and edit toolbar (otherwise the UI would hang once the toolbar is in edit mode).

Co-authored-by: Christian Sadilek <[email protected]>
  • Loading branch information
MozLando and csadilek committed May 20, 2020
2 parents b5b0d31 + cdd2d76 commit f0b900c
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ import mozilla.components.ui.autocomplete.InlineAutocompleteEditText
import mozilla.components.ui.autocomplete.OnFilterListener
import kotlin.coroutines.CoroutineContext

// This is used for truncating URLs to prevent extreme cases from
// slowing down UI rendering e.g. in case of a bookmarklet or a data URI.
// https://github.com/mozilla-mobile/android-components/issues/5249
const val MAX_URI_LENGTH = 25000

/**
* A customizable toolbar for browsers.
*
Expand Down Expand Up @@ -99,7 +104,7 @@ class BrowserToolbar @JvmOverloads constructor(
// We update the display toolbar immediately. We do not do that for the edit toolbar to not
// mess with what the user is entering. Instead we will remember the value and update the
// edit toolbar whenever we switch to it.
display.url = value
display.url = value.take(MAX_URI_LENGTH)
}

override var siteSecure: Toolbar.SiteSecurity
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import mozilla.components.concept.toolbar.AutocompleteDelegate
import mozilla.components.concept.toolbar.Toolbar
import mozilla.components.concept.toolbar.Toolbar.SiteSecurity
import mozilla.components.concept.toolbar.Toolbar.SiteTrackingProtection
import mozilla.components.support.test.argumentCaptor
import mozilla.components.support.test.mock
import mozilla.components.support.test.robolectric.testContext
import org.junit.Assert.assertEquals
Expand Down Expand Up @@ -121,6 +122,30 @@ class BrowserToolbarTest {
verify(edit, never()).updateUrl(ArgumentMatchers.anyString(), ArgumentMatchers.anyBoolean(), ArgumentMatchers.anyBoolean())
}

@Test
fun `displayUrl is truncated to prevent extreme cases from slowing down the UI`() {
val toolbar = BrowserToolbar(testContext)
val display: DisplayToolbar = mock()
val edit: EditToolbar = mock()

toolbar.display = display
toolbar.edit = edit

toolbar.url = "a".repeat(MAX_URI_LENGTH + 1)
toolbar.url = "b".repeat(MAX_URI_LENGTH)
toolbar.url = "c".repeat(MAX_URI_LENGTH - 1)

val urlCaptor = argumentCaptor<String>()
verify(display, times(3)).url = urlCaptor.capture()

val capturedValues = urlCaptor.allValues
// Value was too long and should've been truncated
assertEquals("a".repeat(MAX_URI_LENGTH), capturedValues[0])
// Values should be the same as before
assertEquals("b".repeat(MAX_URI_LENGTH), capturedValues[1])
assertEquals("c".repeat(MAX_URI_LENGTH - 1), capturedValues[2])
}

@Test
fun `last URL will be forwarded to edit toolbar when switching mode`() {
val toolbar = BrowserToolbar(testContext)
Expand Down

0 comments on commit f0b900c

Please sign in to comment.