1813776 - Remove the search string (URL) validation in "Add search engine"

Status: RESOLVED FIXED

(Firefox for Android :: Search, defect, P3)

Description

[Christian Sadilek [:csadilek]]

From github: https://github.com/mozilla-mobile/fenix/issues/25867.

    It's in general pretty much useless and in some cases counter-productive: preventing useful things like "https://translate.google.com/translate?hl=en&sl=auto&tl=en&u=%s" being added.
    Related: #9831, #10605, #11332

┆Issue is synchronized with this Jira Task

Change performed by the Move to Bugzilla add-on.

Comment 1

[BugBot [:suhaib / :marco]]

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

For more information, please visit BugBot documentation.

Comment 2

[Mark Banner (:standard8)]

I think this could use a description of what is wrong with the current validation, and what it prevents.

I think there should be at least some validation to avoid the user putting in something totally random that won't work, e.g. is it a valid URL (if %s is replaced), does it have a %s?

Comment 3

[jackyzy823]

According to https://github.com/mozilla-mobile/fenix/issues/25867#issuecomment-1250358336 , the problem could be related to bug 1850006

Ref: https://github.com/mozilla-mobile/firefox-android/blob/44cd58f9f44c9decb5f16c6442a9b1c6ba4e2c09/fenix/app/src/main/java/org/mozilla/fenix/settings/search/SearchStringValidator.kt#L20

Comment 4

[u758202]

This bug makes it impossible to add search engines like https://nitter.poast.org/search?f=tweets&q=%s&e-nativeretweets=on.

Breaking a feature for everyone to protect a user that will "enter something totally random that won't work" from himself doesn't seem reasonable.

Even if there was an unlikely large number of people entering random texts into the field and then filing bug reports because their random texts aren't working as searches, all you'd have to do was point those people to a support article and close the bug.

Comment 5

[Mark Banner (:standard8)]

When testing the URL, the "add search page" attempts to make a connection to the server. In the case of comment 0 it changes:

• https://translate.google.com/translate?hl=en&sl=auto&tl=en&u=%s
  to
• https://translate.google.com/translate?hl=en&sl=auto&tl=en&u=1

However, Google sees that 1 isn't a web page, and so returns 400, this replicates on desktop as well. If I try it on desktop with changing the 1 to a URL then it works.

In the case of comment 4, I'm not quite sure what is going on. https://nitter.poast.org/search?f=tweets&q=1&e-nativeretweets=on returns 500, but works fine when I load it in a browser.

I'm guessing the intent was to make sure that the server has been typed correctly, so maybe we should relax the check to "any server that returns a response", rather than the server having to return a 2xx response.

Comment 6

[08xjcec48]

This also affects IMDb: https://www.imdb.com/find/?q=%s&s=tt returns 500.

If you still believe that it is important to validate custom search URLs on Android, couldn't you disable the current code altogether until you have enough time to work on a better validation logic?

Comment 7

[Roger Yang [:royang]]

Created Bug 1967794 as a follow up. I will remove URL validation with this issue.

Comment 8

[Roger Yang [:royang]]

Attachment: Bug 1813776 - Remove URL validation in custom search engine settings.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8c4ab0f87eb1
Remove URL validation in custom search engine settings. r=android-reviewers,tchoh,android-l10n-reviewers,delphine

Comment 10

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/8c4ab0f87eb1