Create extension to trim url #12928

sarah541 · 2022-10-11T17:54:43Z

Pull Request checklist

Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
Tests: This PR includes thorough tests or an explanation of why it does not
Changelog: This PR includes a changelog entry or does not need one
Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

After merge

Milestone: Make sure issues closed by this pull request are added to the milestone of the version currently in development.
Breaking Changes: If this is a breaking change, please push a draft PR on Reference Browser to address the breaking issues.

GitHub Automation

Fixes #12804
Fixes #12930

gabrielluong · 2022-10-11T18:46:52Z

components/browser/toolbar/src/main/java/mozilla/components/browser/toolbar/Extensions.kt

+// https://github.com/mozilla-mobile/android-components/issues/5249
+const val MAX_URI_LENGTH = 25000
+
+fun getTrimmedUrl(url: String): String {


I am wondering if this should be moved to one of our more generic support components since this is being used more widely outside this browser-toolbar component. One idea is support-ktx https://searchfox.org/mozilla-mobile/source/android-components/components/support/ktx/src/main/java/mozilla/components/support/ktx/kotlin/String.kt

We also want to make this an actual extension of Strings

Suggested change

fun getTrimmedUrl(url: String): String {

fun String.toTrimmedUrl(): String {

Also, please add a kDoc

I was kinda looking for this. Thank you!

Seems like ktlint complains about all existing problems in the touched files.

Building on Windows with the git config `core.autocrlf=true` (sorry) results in a line ending of `\r\n` in `version.txt`, so just removing `\n` leaves trailing whitespace in the resulting Java code that ends in a build failure.

…/android-components into performance_bug_1790913

Amejia481 · 2022-10-12T20:51:44Z

buildSrc/src/main/java/Gecko.kt

@@ -9,7 +9,7 @@ object Gecko {
    /**
     * GeckoView Version.
     */
-    const val version = "107.0.20221011093208"
+    const val version = "107.0.20221012094509"


nit: Looks like it was committed by mistake :)

Amejia481 · 2022-10-12T20:53:20Z

...nts/feature/app-links/src/main/java/mozilla/components/feature/app/links/AppLinksUseCases.kt

@@ -51,11 +51,19 @@ class AppLinksUseCases(
    private val launchInApp: () -> Boolean = { false },
    private val alwaysDeniedSchemes: Set<String> = ALWAYS_DENY_SCHEMES
 ) {
-    @Suppress("QueryPermissionsNeeded") // We expect our browsers to have the QUERY_ALL_PACKAGES permission
+    @Suppress(


Nit: Looks like unrelated changes

Amejia481 · 2022-10-12T20:53:41Z

components/feature/prompts/src/main/res/values-en-rCA/strings.xml

+    <!-- Title of the time picker dialog. -->
+    <string name="mozac_feature_prompts_set_time">Set time</string>


Same as https://github.com/mozilla-mobile/android-components/pull/12928/files#r993886594

Amejia481 · 2022-10-12T20:53:53Z

...syncedtabs/src/main/java/mozilla/components/feature/syncedtabs/presenter/DefaultPresenter.kt

+        accountManager.unregisterForSyncEvents(eventObserver)
+        accountManager.unregister(accountObserver)


Same as https://github.com/mozilla-mobile/android-components/pull/12928/files#r993886594

Amejia481 · 2022-10-12T20:54:23Z

components/support/ktx/src/main/java/mozilla/components/support/ktx/kotlin/String.kt

+
+/**
+ * Returns truncated urls and search terms to improve performance.
+ */
+fun String.toTrimmedUrl(): String {
+    return this.take(MAX_URI_LENGTH)
+}


Amejia481 · 2022-10-12T20:55:03Z

I think we will need to rebase as there are some unrelated changes that have been leaked to the PR :)

create extension to trim url

58eb043

sarah541 requested a review from Amejia481 October 11, 2022 17:54

sarah541 requested a review from a team as a code owner October 11, 2022 17:54

gabrielluong reviewed Oct 11, 2022

View reviewed changes

mozilla-l10n-automation-bot and others added 8 commits October 12, 2022 00:20

Import l10n.

3dcb99f

For mozilla-mobile#12804 - Catch the exception when querying activities

39b45b3

Update GeckoView (Nightly) to 107.0.20221012094509.

33d3b6d

For mozilla-mobile#12930 - Unregister observers to prevent memory leaks

5aa1a19

For mozilla-mobile#12930 - Fix old but now blocking ktlint issues

19c4186

Seems like ktlint complains about all existing problems in the touched files.

Fix gradle version code generation on Windows

b281cf5

Building on Windows with the git config `core.autocrlf=true` (sorry) results in a line ending of `\r\n` in `version.txt`, so just removing `\n` leaves trailing whitespace in the resulting Java code that ends in a build failure.

Merge branch 'performance_bug_1790913' of https://github.com/sarah541…

9f14d5f

…/android-components into performance_bug_1790913

Create extension to trim url and search terms

03b99e7

sarah541 requested review from jonalmeida and rocketsroger as code owners October 12, 2022 18:54

sarah541 removed request for jonalmeida and rocketsroger October 12, 2022 18:55

Amejia481 reviewed Oct 12, 2022

View reviewed changes

sarah541 closed this Oct 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create extension to trim url #12928

Create extension to trim url #12928

sarah541 commented Oct 11, 2022 •

edited by github-actions bot

Loading

gabrielluong Oct 11, 2022

gabrielluong Oct 11, 2022

Amejia481 Oct 11, 2022

sarah541 Oct 12, 2022

Amejia481 Oct 12, 2022 •

edited

Loading

Amejia481 Oct 12, 2022

Amejia481 Oct 12, 2022

Amejia481 Oct 12, 2022

Amejia481 Oct 12, 2022

Amejia481 commented Oct 12, 2022

	fun getTrimmedUrl(url: String): String {
	fun String.toTrimmedUrl(): String {

		<!-- Title of the time picker dialog. -->
		<string name="mozac_feature_prompts_set_time">Set time</string>

		accountManager.unregisterForSyncEvents(eventObserver)
		accountManager.unregister(accountObserver)

Create extension to trim url #12928

Create extension to trim url #12928

Conversation

sarah541 commented Oct 11, 2022 • edited by github-actions bot Loading

Pull Request checklist

After merge

GitHub Automation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Amejia481 Oct 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Amejia481 commented Oct 12, 2022

sarah541 commented Oct 11, 2022 •

edited by github-actions bot

Loading

Amejia481 Oct 12, 2022 •

edited

Loading