Leo search engine broken, searches for {searchTerms} instead of search terms

VERIFIED FIXED in Firefox 68

Status

()

defect
P1
normal
VERIFIED FIXED
3 months ago
Yesterday

People

(Reporter: Pike, Assigned: standard8)

Tracking

(Regression, {regression})

unspecified
Firefox 68
Points:
2

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox66 unaffected, firefox67 unaffected, firefox68+ verified)

Details

Attachments

(1 attachment)

I just noticed that using our dict.leo.org search plugin is broken, I guess it regressed by bug 1496075.

This might be from having {searchTerms} in the search_url, if so there's a couple that are affected, https://dxr.mozilla.org/mozilla-central/search?q=regexp%3Asearch_url%22.*searchTerms+path%3Asearch%2Fext&redirect=false

I didn't check the localized URLs.

Bugbug thinks this bug is a task, but please change it back in case of error.

Type: defect → task

I think the bot is wrong

Type: task → defect
Flags: needinfo?(dharvey)

I said on irc I'll take a look at this.

Assignee: nobody → standard8
Flags: needinfo?(dharvey)

Some notes so that I remember next week:

  • It looks like the Addon Manager is giving us the URLs from the extensions as encoded.
  • We probably want to decode before the URL gets into the SearchEngine system, as that will likely match the other URLs (though maybe check this against open search).
  • Need to check what we're actually storing in the cache (is it the partial or full submission url).
  • Need to check that if we fix it, then nightly users who are already using the system will be correctly updated.

I am not sure we would want to decode would we? we can change the AddonManager or OpenSearch code to give us consistently encoded or unencoded URLs right?

Status: NEW → ASSIGNED
Priority: -- → P1

(In reply to Dale Harvey (:daleharvey) from comment #5)

I am not sure we would want to decode would we? we can change the AddonManager or OpenSearch code to give us consistently encoded or unencoded URLs right?

I just experimented a bit and found out what's happening. In Schemas.jsm when we hit a URL, it is doing:

let url = new URL(string).href;

In a web console, this gives us:

>> new URL("https://test.com/?q={abc}").href
"https://test.com/?q={abc}"
>> new URL("https://test.com/{abc}").href
"https://test.com/%7Babc%7D"

So basically if there's no query parameter then the curly brackets force encoding. I'm going to assume that new URL() is working to the spec.

This is why most of our URLs work, because we have them as actual query parameters.

I think ideally within SearchService we should work with unencoded URLs. Since that's what we've been doing up until now and it hasn't caused us an issue, and it is easier.

I have no idea if changing the Add-on Manager will adversely affect other things. Unfortunately I suspect these types of URLs aren't very well tested. Therefore, I'm thinking about what seems to be the safer option of translating them when we get them from the add-on manager into the search service.

Points: --- → 2

Can we just split these engines to use params and punt on this?

Are there any engines that require {searchTerms} in the URL and don't use params?

At least dict.leo.org doesn't seem to have a param-way to search. Even if I customize the search options, the options are params, but the search term is still part of the URL path.

(In reply to Mike Kaply [:mkaply] from comment #7)

Can we just split these engines to use params and punt on this?

This is a one-liner effectively (plus a small test), so I'm really not too worried about it.

Are there any engines that require {searchTerms} in the URL and don't use params?

Looking at the list of possibilities, Dale's already removed some in bug 1545517. The ones remaining that are working and require {searchTerms} in the URL are:

leo_ende_de (https://dict.leo.org/englisch-deutsch/)
pwn-pl (https://encyklopedia.pwn.pl/szukaj/)

I'm guessing at least the first is used occasionally, given this bug ;-)

Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/38c57ccca71e
Handle Search Engine urls properly where the search terms are in the url rather than as parameters. r=daleharvey
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68

Mark, is there anything manual QA can do to help test this?

Flags: qe-verify?
Flags: needinfo?(standard8)
  1. Download Firefox 68.0 beta in German from https://www.mozilla.org/en-US/firefox/beta/all/
  2. Install and launch it.
  3. Type an English word into the search box.
  4. Click into the search box.
  5. In the list of search engines shown below, click in the second row on the right-most search engine (German and UK flag as background with an 'L').

Expected: A page with search results should appear.

Flags: needinfo?(standard8)
Flags: qe-verify? → qe-verify+

Hi,

I tested this issue on Windows 10 and Ubuntu 18.04 with an old Nightly DE build 68.0a1 (20190418221600) and I was able to reproduce it.
I also tested it using the latest Nightly DE build 69.0a1 (2019-06-26) and latest Beta DE version 68.0b13 (20190624133534) and I'm not able to reproduce the issue. Based on this I will mark the bug as Verified.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Iteration: --- → 68.3 - Apr 15 - 28
You need to log in before you can comment on or make changes to this bug.