Closed Bug 1320712 Opened 3 years ago Closed 3 years ago

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

Categories

(Firefox :: Search, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 53
Tracking Status
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: flod, Assigned: mkaply)

Details

Attachments

(2 files)

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
Summary: Amazon.com (amazondotcom.xml) is broken for UTF-8 searches → Amazon.com default searchplugin (amazondotcom.xml) is broken for UTF-8 searches
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.
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.
I think adding the ie=UTF-8 makes more sense than setting the encoding in the engine. I'll do a patch.
This works really well. You can even search (and find) japanese items in the various country specific Amazons.
Attachment #8815068 - Flags: review?(francesco.lodolo)
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.
Attachment #8815068 - Flags: review?(francesco.lodolo) → review+
Assignee: nobody → mozilla
> 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
Attached patch New patchSplinter Review
You're right. We should stay close to the old.

This adds back the input encoding and uses it for the ie value.
Attachment #8815346 - Flags: review?(francesco.lodolo)
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
Attachment #8815346 - Flags: review?(francesco.lodolo) → review+
https://hg.mozilla.org/mozilla-central/rev/3a6e867880da
https://hg.mozilla.org/mozilla-central/rev/d254eb61360d
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
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
Attachment #8815346 - Flags: approval-mozilla-aurora?
(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?
Flags: needinfo?(mozilla)
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.
Flags: needinfo?(mozilla)
Comment on attachment 8815346 [details] [diff] [review]
New patch

use utf-8 for amazon search, take in aurora52

Thanks for the clarification Mike.
Attachment #8815346 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Setting qe-verify- based on Mike's assessment on manual testing needs (Comment 12) and the fact that this fix has automated coverage.
Flags: qe-verify-
It works on windows 7 64 bit.
You need to log in before you can comment on or make changes to this bug.