The RSS subscribe.js should not double-decoded the URL passed in....

The RSS subscribe.js should not double-decoded the URL passed in.

This causes feeds from Twitter searches to break because the hash
sign (%23) gets decoded before passed to Twitter.

BUG=70226
TEST=ExtensionBrowserTest.ParseFeedInvalidFeed4


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=72122

[Finnur, 2011-01-20 14:56:46]

http://codereview.chromium.org/6332008/diff/1/chrome/test/data/extensions/sub...
File chrome/test/data/extensions/subscribe_page_action/subscribe.js (right):

http://codereview.chromium.org/6332008/diff/1/chrome/test/data/extensions/sub...
chrome/test/data/extensions/subscribe_page_action/subscribe.js:10: var feedUrl =
decodeURIComponent(queryString[0]);
We already decode the feedUrl here, which is why we don't need to do it again on
line 108.

[Finnur, 2011-01-20 15:12:19]

Adding Jochen to CC list.

On 2011/01/20 14:56:46, Finnur wrote:
>
http://codereview.chromium.org/6332008/diff/1/chrome/test/data/extensions/sub...
> File chrome/test/data/extensions/subscribe_page_action/subscribe.js (right):
> 
>
http://codereview.chromium.org/6332008/diff/1/chrome/test/data/extensions/sub...
> chrome/test/data/extensions/subscribe_page_action/subscribe.js:10: var feedUrl
=
> decodeURIComponent(queryString[0]);
> We already decode the feedUrl here, which is why we don't need to do it again
on
> line 108.

[Aaron Boodman, 2011-01-20 17:47:15]

http://codereview.chromium.org/6332008/diff/1/chrome/test/data/extensions/sub...
File chrome/test/data/extensions/subscribe_page_action/subscribe.js (left):

http://codereview.chromium.org/6332008/diff/1/chrome/test/data/extensions/sub...
chrome/test/data/extensions/subscribe_page_action/subscribe.js:42: "%s",
escape(encodeURI(feedUrl)));
escape and encodeURI do subtlety different things. There is also
encodeURIComponent(). Which one do you want here? I sort of think you probably
want encodeURIComponent().

[Finnur, 2011-01-20 21:48:12]

Yeah, I think you are right. Updated.

On 2011/01/20 17:47:15, Aaron Boodman wrote:
>
http://codereview.chromium.org/6332008/diff/1/chrome/test/data/extensions/sub...
> File chrome/test/data/extensions/subscribe_page_action/subscribe.js (left):
> 
>
http://codereview.chromium.org/6332008/diff/1/chrome/test/data/extensions/sub...
> chrome/test/data/extensions/subscribe_page_action/subscribe.js:42: "%s",
> escape(encodeURI(feedUrl)));
> escape and encodeURI do subtlety different things. There is also
> encodeURIComponent(). Which one do you want here? I sort of think you probably
> want encodeURIComponent().

[Aaron Boodman, 2011-01-20 23:01:03]

lgtm