427304 - Cannot search with non-ascii characters (nsURLFormatter wrongly parses encoded uri)

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Kohei Yoshino [:kohei]]

Attachment: fix

Steps to Reproduce:

1. Enable HTTP Logging (http://developer.mozilla.org/en/docs/HTTP_Logging)
2. Run Firefox
3. Open Add-ons Manager
4. Type タブ in the search box
5. Type ツールバー in the search box
6. Check the log file for the search URLs

Actual Results:

https://services.addons.mozilla.org/en-US/firefox/api/1/search/%E3%82E3%83%96/all/10/Darwin/3.0pre
https://services.addons.mozilla.org/en-US/firefox/api/1/search/%E3%83%84%E3%83E3%83E3%83%90%E3%83%BC/all/10/Darwin/3.0pre

Expected Results:

https://services.addons.mozilla.org/en-US/firefox/api/1/search/%E3%82%BF%E3%83%96/all/10/Darwin/3.0pre
https://services.addons.mozilla.org/en-US/firefox/api/1/search/%E3%83%84%E3%83%BC%E3%83%AB%E3%83%90%E3%83%BC/all/10/Darwin/3.0pre

Additional Information:

nsURLFormatter.js formatURL() doesn't consider encoded uri.

Comment 1

[Kohei Yoshino [:kohei]]

Attachment: fix for 1.8 branch

Comment 2

[Dave Townsend [:mossop]]

Comment on attachment 313861 [details] [diff] [review]
fix

I am not a valid reviewer for this component. Maybe Gavin?

My only thought would be maybe the regex should only match sequences longer than 2 characters but that would break the ID case anyway.

Also it could do with a unit test I think.

Comment 3

[Kohei Yoshino [:kohei]]

Here is a dump in the formatURL():

aMatch: %LOCALE%
aMatch: %LOCALE%
aMatch: %APP%
aMatch: %LOCALE%
aMatch: %APP%
aMatch: %BF% <- should not be cut off 
aMatch: %OS%
aMatch: %VERSION%
aMatch: %LOCALE%
aMatch: %BF% <- ditto.
aMatch: %OS%
aMatch: %VERSION%

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 313861 [details] [diff] [review]
fix

Please add a test for this to toolkit/components/urlformatter/tests/unit/test_urlformatter.js .

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 313861 [details] [diff] [review]
fix

a=beltzner, please do file that test as well

Comment 6

[Dave Townsend [:mossop]]

What is happening with this, why has it not landed yet?

Comment 7

[u217243]

Attachment: unit test

Nakano-san, please check-in this and the patch for me.

Comment 8

[u217243]

Attachment: unit test, fixed

Added a missing dot in encodedUrlRef.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

checked-in the patch and the test.

Comment 10

[Kohei Yoshino [:kohei]]

Verified.

Note that the search with non-ascii chars is still broken due to bug 415317.

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008042404 Minefield/3.0pre