1683991 - Built-in eBay search engine just sends people to ebay.com when there are multiple words in a search query

Status: VERIFIED FIXED

(Firefox :: Search, defect, P2)

Description

[lolrepeatlol]

Attachment: OCDpgmHLLs.gif

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:86.0) Gecko/20100101 Firefox/86.0

Steps to reproduce:

I used tab-to-search to search eBay, and typed in a multi-word search such as "apple watch."

Actual results:

I got sent to ebay.com.

Expected results:

I should have gotten sent to the search page on eBay for that search query.

Tested on Firefox 84, reproducible on Firefox Nightly 86.

Comment 1

[Mark Banner (:standard8)]

Thank you for the report, I can confirm this. It also happens in the Firefox 77.

As far as I can tell it is an eBay issue. We'll raise it with them. I'll leave this open for tracking purposes.

Comment 2

[Gene Wood [:gene]]

I can also confirm this is happening in Firefox 84.0 in a brand new profile. This is likely affecting our revenue share.

Firefox sends the user to

https://rover.ebay.com/rover/1/711-53200-19255-0/1?ff3=4&toolid=20004&campid=5338192028&customid=&mpre=https://www.ebay.com/sch/test+test where test test is the search query.

Which responds with a HTTP/1.1 301 Moved Permanently sending the user to

http://www.ebay.com/

Comment 3

[Mark Banner (:standard8)]

As I said above, we have already raised it with eBay and are currently discussing with them. I think we should have resolution next week.

Comment 4

[Dale Harvey (:daleharvey)]

Attachment: Bug 1683991 - Update ebay urls to new search endpoint.

Comment 5

[Dale Harvey (:daleharvey)]

So I have some updates @ https://phabricator.services.mozilla.com/D100609 to URL's that are working for me

However a little worried we are going to have a problem with updates here, the searchUrlGetParams will be taken from remoteSettings (we should take them out of the webextension when they are duped, which they currently are) and the get params are not compatible with ebay.com/sch/ vs rover.ebay.com, which means if we update remotesettings it will break for people who havent yet got an update webextention. I feel like we did think about this / have provisions for this situation previously, but I cant remember them.

One possible solution may be to remove the get param definitions from remote settings if we can so they are picked up from the extension, then users will get the get param updates the same time as the domain update and so no mismatch, this however has the problem that we cant / may not be able push out immediate updates to the extensions yet

Comment 6

[jscher2000]

Would it work as a general approach to replace each space in {searchTerms} with a + character? I think that's a very common syntax for search engine URLs, but I suppose it might be a negative for something out there.

Comment 7

[Mark Banner (:standard8)]

(In reply to jscher2000 from comment #6)

Would it work as a general approach to replace each space in {searchTerms} with a + character? I think that's a very common syntax for search engine URLs, but I suppose it might be a negative for something out there.

We already do that - see the link in comment 2.

Comment 8

[Dale Harvey (:daleharvey)]

[Tracking Requested - why for this release]:

We have a fix awaiting confirmation from ebay, the ebay search experience will be broken for users so would like to get fixed asap

Comment 9

[Julien Cristau [:jcristau]]

We're out of beta builds for 85, so a fix would need to be ready very soon to get into the RC.

Comment 10

[Dale Harvey (:daleharvey)]

Apologies we have been waiting on confirmation from the partner and just received it this minute, its on autoland now

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d7b14690ec5f
Update ebay urls to new search endpoint. r=Standard8

Comment 12

[Noemi Erli[:noemi_erli]]

Backed out changeset d7b14690ec5f (Bug 1683991) for causing failures in browser_searchEngine_behaviors.js CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&classifiedState=classified&revision=d7b14690ec5ff2e9bdc02c6043772a6db922827e&searchStr=linux%2C18.04%2Cx64%2Cdebug%2Cmochitests%2Ctest-linux1804-64%2Fdebug-mochitest-browser-chrome-e10s%2Cbc3

Failure log: https://treeherder.mozilla.org/logviewer?job_id=326861191&repo=autoland&lineNumber=8598

Backout: https://hg.mozilla.org/integration/autoland/rev/3f37d2333f01301190453eb186b177f2010b318c

Comment 13

[Dale Harvey (:daleharvey)]

Forgot about that test, fixed is green @ https://treeherder.mozilla.org/jobs?repo=try&revision=199ca10522e6831da1f3fc178e2d3248c52929ff

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f5ba8542e6f7
Update ebay urls to new search endpoint. r=Standard8

Comment 15

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/f5ba8542e6f7

Comment 16

[Julien Cristau [:jcristau]]

Please request uplift to mozilla-release

Comment 17

[Dale Harvey (:daleharvey)]

Comment on attachment 9195087 [details]
Bug 1683991 - Update ebay urls to new search endpoint.

Beta/Release Uplift Approval Request

• User impact if declined: Ebay engine will be broken when searching with multiple words
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Its a change to already broken search engine urls
• String changes made/needed:

Comment 18

[Julien Cristau [:jcristau]]

Comment on attachment 9195087 [details]
Bug 1683991 - Update ebay urls to new search endpoint.

approved for 85 rc1

Comment 19

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-release/rev/c483081823c9

Comment 20

[Petruta Horea [:phorea], Desktop QA]

We performed some checks using latest Nightly 86.0a1 2021-01-18 across OS's, including:

• eBay searches with multiple word entries
• eBay searches on various locales/regions
• eBay codes are correctly generated

We also checked that on Fx versions without the fix, the searches are correctly performed and the redirects are still using rover

Comment 21

[Petruta Horea [:phorea], Desktop QA]

Verified as fixed using Firefox 85.0RC under Win 10 64-bit and Ubuntu 18.04 64-bit.

Comment 22

[Dale Harvey (:daleharvey)]

Comment on attachment 9195087 [details]
Bug 1683991 - Update ebay urls to new search endpoint.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Broken built in search engine
• User impact if declined: Users searchhing for terms with multiple words will lose their search terms
• Fix Landed on Version: 78
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Url changes only to already broken engine
• String or UUID changes made by this patch:

Comment 23

[Ryan VanderMeulen [:RyanVM]]

This needs a rebased patch for ESR78.

Comment 24

[Dale Harvey (:daleharvey)]

Attachment: Bug 1683991 - Update ebay urls to new search endpoint on ESR.

Comment 25

[Dale Harvey (:daleharvey)]

Apologies for the delay, thats it up on phab (https://phabricator.services.mozilla.com/D104566) and passing tests here (the conflict was due to suggestions being added for ebay)

Comment 26

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9202129 [details]
Bug 1683991 - Update ebay urls to new search endpoint on ESR.

Approved for 78.8esr, thanks for the rebased patch.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/55097a5cf353

Comment 28

[Petruta Horea [:phorea], Desktop QA]

Verified fixed on Firefox 78.8esr build 2 under Win 10 64-bit and Mac OSX 10.15.