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

chrome/test/data/extensions/subscribe_page_action/subscribe.js

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Grab the querystring, removing question mark at the front and splitting on
 // the ampersand.
 var queryString = location.search.substring(1).split("&");
 
 // The feed URL is the first component and always present.
 var feedUrl = decodeURIComponent(queryString[0]);

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

We already decode the feedUrl here, which is why we don't need to do it again on
line 108.

 
 // We allow synchronous requests for testing. This component is only present
 // if true.
 var synchronousRequest = queryString[1] == "synchronous";
 
 // The XMLHttpRequest object that tries to load and parse the feed, and (if
 // testing) also the style sheet and the frame js.
 var req;
 
 // Depending on whether this is run from a test or from the extension, this
@@ skipping 11 matching lines @@
 
 // A list of feed readers, populated by localStorage if available, otherwise
 // hard coded.
 var feedReaderList;
 
 // Navigates to the reader of the user's choice (for subscribing to the feed).
 function navigate() {
   var select = document.getElementById('readerDropdown');
   var url =
       feedReaderList[select.selectedIndex].url.replace(
-          "%s", escape(encodeURI(feedUrl)));

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

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().

+          "%s", escape(feedUrl));
 
   // Before we navigate, see if we want to skip this step in the future...
   if (storageEnabled) {
     // See if the user wants to always use this reader.
     var alwaysUse = document.getElementById('alwaysUse');
     if (alwaysUse.checked) {
       window.localStorage.defaultReader =
           feedReaderList[select.selectedIndex].url;
       window.localStorage.showPreviewPage = "No";
     }
@@ skipping 45 matching lines @@
     frameScript = "<script>" + req.responseText +
                     "<" + "/script>";
   } else {
     // Normal loading just requires links to the css and the js file.
     styleSheet = "<link rel='stylesheet' type='text/css' href='" +
                     chrome.extension.getURL("style.css") + "'>";
     frameScript = "<script src='" + chrome.extension.getURL("iframe.js") +
                     "'></" + "script>";
   }
 
-  feedUrl = decodeURIComponent(feedUrl);
   req.onload = handleResponse;
   req.onerror = handleError;
   // Not everyone sets the mime type correctly, which causes handleResponse
   // to fail to XML parse the response text from the server. By forcing
   // it to text/xml we avoid this.
   req.overrideMimeType('text/xml');
   req.open("GET", feedUrl, !synchronousRequest);
   req.send(null);
 
   document.getElementById('feedUrl').href = 'view-source:' + feedUrl;
@@ skipping 97 matching lines @@
 function onSelectChanged() {
   if (!storageEnabled)
     return;
   var readerDropdown = document.getElementById('readerDropdown');
 
   // If the last item (Manage...) was selected we show the options.
   var oldSelection = readerDropdown.selectedIndex;
   if (readerDropdown.selectedIndex == readerDropdown.length - 1)
     window.location = "options.html";
 }