1320712 - Amazon.com default searchplugin (amazondotcom.xml) is broken for UTF-8 searches

Status: RESOLVED FIXED

(Firefox :: Search, defect)

Description

[Francesco Lodolo [:flod]]

Just noticed while working on bug 1304757 that the Italian searchplugin is set to encoding ISO-8859-1, while the English one is set to UTF-8.

So, I've installed the en-US one manually, and tried to search for "caffè", the 'è' in mingled, both in the URL and the search field.

https://www.amazon.com/s?ie=UTF8&field-keywords=caff%C3%83%C2%A8&index=blended&link_code=qs&sourceid=Mozilla-search&tag=mozilla-20

Comment 1

[Mike Kaply [:mkaply]]

Odd. It looks like the URL we use for search maps incorrectly.

If you go t:

https://www.amazon.com/exec/obidos/external-search/?field-keywords=caffè

it doesn't work (and adds the UTF-8 in).

If you go to

https://www.amazon.com/s?field-keywords=caffè

It works fine.

Comment 2

[Francesco Lodolo [:flod]]

Interesting. If we pass ie=UTF-8 in the searchplugin it seems to work.
https://l10n.mozilla-community.org/~flod/testsp/
https://l10n.mozilla-community.org/~flod/testsp/amazondotcom.xml

Haven't tested if it means that we can switch all other locales too.

Comment 3

[Mike Kaply [:mkaply]]

I think adding the ie=UTF-8 makes more sense than setting the encoding in the engine. I'll do a patch.

Comment 4

[Mike Kaply [:mkaply]]

Attachment: Pass UTF-8 as the input encoding

This works really well. You can even search (and find) japanese items in the various country specific Amazons.

Comment 5

[Francesco Lodolo [:flod]]

Comment on attachment 8815068 [details] [diff] [review]
Pass UTF-8 as the input encoding

Review of attachment 8815068 [details] [diff] [review]:
-----------------------------------------------------------------

Interesting, it's the first time I see us dropping the <InputEncoding> declaration. Is there any good reason for doing it?

See Google for example, where we have it
https://hg.mozilla.org/mozilla-central/file/tip/browser/locales/searchplugins/google.xml#l8

Anyhow, I tested the English searchplugin and it works correctly.

Comment 6

[Mike Kaply [:mkaply]]

> Interesting, it's the first time I see us dropping the <InputEncoding> declaration. Is there any good reason for doing it?

It defaults to UTF-8, so we technically don't need it.

https://dxr.mozilla.org/mozilla-central/source/toolkit/components/search/nsSearchService.js#162

Interesting enough, instead of hardcoding UTF-8 in the params, we can actually use {inputEncoding}

https://dxr.mozilla.org/mozilla-central/source/toolkit/components/search/nsSearchService.js#155

Comment 7

[Mike Kaply [:mkaply]]

Attachment: New patch

You're right. We should stay close to the old.

This adds back the input encoding and uses it for the ie value.

Comment 8

[Francesco Lodolo [:flod]]

Comment on attachment 8815346 [details] [diff] [review]
New patch

Review of attachment 8815346 [details] [diff] [review]:
-----------------------------------------------------------------

Yep, this works well and is less scary than dropping the inputencoding. Tested en-US and it

Comment 9

[Mike Kaply [:mkaply]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3a6e867880dae0d26767bf1653731e92b3e8789d
Bug 1320712 - Make UTF-8 searches work on Amazon. r=flod

Comment 10

[Mike Kaply [:mkaply]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d254eb61360d3c20172eb7aba1154e2779850263
Bug 1320712 - Add new parameters to Amazon tests. r=bustage

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/3a6e867880da
https://hg.mozilla.org/mozilla-central/rev/d254eb61360d

Comment 12

[Mike Kaply [:mkaply]]

Comment on attachment 8815346 [details] [diff] [review]
New patch

Approval Request Comment
[Feature/Bug causing the regression]: Enable UTF-8 search for Amazon
[User impact if declined]: Incorrect search results for certain products.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Adds additional parameter to search. Doesn't affect existing search
[String changes made/needed]: None

Comment 13

[Julien Cristau [:jcristau]]

(In reply to Mike Kaply [:mkaply] from comment #12)
> [Feature/Bug causing the regression]: Enable UTF-8 search for Amazon

not sure this answers the question, which is "if this is a regression, when was it introduced?".  Was this working before bug 1276740?

Comment 14

[Mike Kaply [:mkaply]]

I was told that was an either or not just a regression line. 

"Feature, or bug # if it is a regression"

So I answered it as feature. 

But to answer your question, this hasn't worked in a while, we think amazon changed something.

Comment 15

[Julien Cristau [:jcristau]]

Comment on attachment 8815346 [details] [diff] [review]
New patch

use utf-8 for amazon search, take in aurora52

Thanks for the clarification Mike.

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/dc0919d69eef
https://hg.mozilla.org/releases/mozilla-aurora/rev/50fa80ba9f88

Comment 17

[Andrei Vaida [:avaida]]

Setting qe-verify- based on Mike's assessment on manual testing needs (Comment 12) and the fact that this fix has automated coverage.

Comment 18

[kavya]

It works on windows 7 64 bit.