1803465 - Fenix looks up ipv6 addresses with search instead of opening them

Status: RESOLVED FIXED

(Fenix :: Toolbar, defect)

Description

[Djfe]

User Agent: Mozilla/5.0 (Android 13; Mobile; rv:107.0) Gecko/107.0 Firefox/107.0

Steps to reproduce:

I copy and pasted this ipv6 url into the address bar of Firefox on Android:
http://[2a03:2260:3006:2:16cc:20ff:fe62:f6a0]/cgi-bin/status

Actual results:

Fenix googled it

Expected results:

Fenix should've accessed the ipv6 address and opened that.

If I click the link on this page, then Fenix opens it just fine.
https://map.aachen.freifunk.net/#!/de/map/14cc2062f6a0

Example why entering ipv6 addresses manually can be useful:
Opening the ipv6 address [fdac:ac] in the browser forwards you to the ipv6 of the mesh node/router you are connected to at that time. (you need to be connected to a Freifunk AP from Freifunk Rheinland for this to even work, this is just an example)

This works fine in Chrome on Android.

Fenix Release and Nightly are affected.

I don't know what the affected component is, so I left it on as General.

Comment 1

[Frederik Braun [:freddy]]

I can confirm that Fenix will believe this is a search.
Moving over to the right component, as it's likely a bug in their address bar code (Fenix or android components - I don't know).

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:cpeterson, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 3

[David A. Madore]

Confirming that this bug is present in Firefox mobile 114.0b9

Comment 4

[jackyzy823]

There was a PR aimed to fix this issue but it was rejected by csadilek due to performance issue of WebURLFinder.

Since It looks like that WebURLFinder have been totally refactored ( and no longer related ? ), Can URLStringUtils part be picked to fix this issue?

From

  "^\\s*(\\w+-+)*\\w+(://[/]*|:|\\.)(\\w+-+)*\\w+([\\S&&[^\\w-]]\\S*)?\\s*$",

To

  "^\\s*(\\w+-+)*\\w+(://[/]*|:|\\.)(\\w+-+)*\\[?[\\w:]+([\\S&&[^\\w-]]\\S*)?\\s*$",

Comment 5

[Christian Sadilek [:csadilek]]

Can URLStringUtils part be picked to fix this issue?

Yes, I think so. I would still request to create a profile before/after to make sure typing and navigating via the toolbar is not affected, but this was not the problematic part.

We have since removed the unnecessary and extensive regex in this PR.

And from the original patch it was this regex that was problematic.

Comment 6

[Paul Marks]

This Lovecraftian horror works for me:

diff --git a/android-components/components/support/utils/src/main/java/mozilla/components/support/ktx/util/URLStringUtils.kt b/android-components/components/support/utils/src/main/java/mozilla/components/support/ktx/util/URLStringUtils.kt
index 755b99e8cf..3e155687d4 100644
--- a/android-components/components/support/utils/src/main/java/mozilla/components/support/ktx/util/URLStringUtils.kt
+++ b/android-components/components/support/utils/src/main/java/mozilla/components/support/ktx/util/URLStringUtils.kt
@@ -77,7 +77,7 @@ object URLStringUtils {
         // www.c-c-
         // 3-3
         Pattern.compile(
-            "^\\s*(\\w+-+)*\\w+(://[/]*|:|\\.)(\\w+-+)*\\w+([\\S&&[^\\w-]]\\S*)?\\s*$",
+            "^\\s*((\\w+-+)*\\w+(://[/]*|:|\\.)(\\w+-+)*\\w+|(\\w+(://[/]*|:))?\\[[0-9A-Fa-f:.]+\\])([\\S&&[^\\w-]]\\S*)?\\s*$",
             flags,
         )
     }

The new IPv6 term (\\w+(://[/]*|:))?\\[[0-9A-Fa-f:.]+\\] is a complete alternative to (\\w+-+)*\\w+(://[/]*|:|\\.)(\\w+-+)*\\w+, because stuffing IPv6 into there seems impossible.

With this change, I can type all of these into the URL bar:

• https://[2606:4700:4700::1111]/
• https:[2606:4700:4700::1111]
• [2606:4700:4700::1111]

Autocomplete is still broken. It suggests [2606/ when typing the URL a second time.

I also found a stale copy of this regex, but I'm not sure what it does:
https://github.com/mozilla-mobile/android-components/blob/bb489ef682b33e8ebc770908bfe06befb13b0dd6/components/support/utils/src/main/java/mozilla/components/support/utils/WebURLFinder.kt#L78

Comment 7

[Paul Marks]

because stuffing IPv6 into there seems impossible.

"Once you free your mind about a concept of harmony and of music being correct, you can do whatever you want."

This solution is more in line with comment 4, while also allowing [::] at the beginning of the string:

diff --git a/android-components/components/support/utils/src/main/java/mozilla/components/support/ktx/util/URLStringUtils.kt b/android-components/components/support/utils/src/main/java/mozilla/components/support/ktx/util/URLStringUtils.kt
index 755b99e8cf..31a28b8d42 100644
--- a/android-components/components/support/utils/src/main/java/mozilla/components/support/ktx/util/URLStringUtils.kt
+++ b/android-components/components/support/utils/src/main/java/mozilla/components/support/ktx/util/URLStringUtils.kt
@@ -77,7 +77,7 @@ object URLStringUtils {
         // www.c-c-
         // 3-3
         Pattern.compile(
-            "^\\s*(\\w+-+)*\\w+(://[/]*|:|\\.)(\\w+-+)*\\w+([\\S&&[^\\w-]]\\S*)?\\s*$",
+            "^\\s*(\\w+-+)*[\\w\\[]+(://[/]*|:|\\.)(\\w+-+)*[\\w\\[:]+([\\S&&[^\\w-]]\\S*)?\\s*$",
             flags,
         )
     }

It works by expanding \w to include [ in the first location, and [: in the second location. The second location could use the syntax from comment 4, but it seems more consistent to use character classes for both.

Let's see if I can figure out how to add unit tests.

Comment 8

[BMO Github Automation]

Attachment: [mozilla-mobile/firefox-android] Bug 1803465 - extend isURLLenient to match IPv6 literals (#4090)

Comment 9

[Paul Marks]

The wonky autocomplete seems to be coming from DomainMatcher.kt

https://github.com/mozilla-mobile/firefox-android/blob/3596bbc5d22812f2593d603e2bf9beed46149eba/android-components/components/support/utils/src/main/java/mozilla/components/support/utils/DomainMatcher.kt#L79

private fun matchSegment(query: String, rawUrl: String): String? {
    if (rawUrl.startsWith(query)) {
        return rawUrl
    }

    val url = Uri.parse(rawUrl)
    Log.d("matchSegment", "url=$url url.host=${url.host}")

10-14 17:34:54.458 5724 5995 D matchSegment: url=https://[2606:4700:4700::1111]/ url.host=[2606

Looks like this bug: https://www.ctrl.blog/entry/android-uri-ipv6.html

Comment 10

[Paul Marks]

./mach android-emulator uses Android 7 (API 24).
The autocomplete bug is fixed when I use an API 34 emulator, so I recommend ignoring it.

https://github.com/mozilla-mobile/firefox-android/pull/4090 should fix everything else.

Comment 11

[BMO Github Automation]

Authored by https://github.com/pmarks-net
https://github.com/mozilla-mobile/firefox-android/commit/ec7b003ee2ef29345183f148603cb5d5a99f7e74
[main] Bug 1803465 - extend isURLLenient to match IPv6 literals (#4090)

Comment 13

[Lorand Janos]

I managed to reproduce the issue on the latest Beta 121.0b2.

Verified as fixed on the latest Nightly build (122.0a1 from 2023-11-24).

I tried the example given in the description, which on Beta, or earlier Nightly versions searched after the address.

Though the page is not connected because the site is temporarily unavailable (or too busy), according to the error message, it is no longer searched, and attempted to load as a webpage.

Devices used: Samsung Galaxy S23 Ultra (Android 13) and Google Pixel 7 (Android 14).

Marking the ticket as verified.

Comment 15

[Paul Marks]

Firefox 122 (with the IPv6 literals fix) is now live on the Play store.