Bug 1128757 - Do not trim fragments from URLs loaded in reader view. r=bnicholson

--HG--
extra : rebase_source : 95965e5fba02193d5bbb94e5427f7d939547b01a

browser/base/content/content.js

@@ -520,8 +520,8 @@ let AboutReaderListener = {
 
           // The loaded page may have changed while we were parsing the document.
           // Make sure we've got the current one.
-          let currentURL = Services.io.newURI(content.document.documentURI, null, null).specIgnoringRef;
-          if (article.url !== currentURL) {
+          let url = Services.io.newURI(content.document.documentURI, null, null).spec;
+          if (article.url !== url) {
             return;
           }
 

browser/modules/ReaderParent.jsm

@@ -178,8 +178,7 @@ let ReaderParent = {
     }
 
     // Next, try to find a parsed article in the cache.
-    let uri = Services.io.newURI(url, null, null);
-    article = yield ReaderMode.getArticleFromCache(uri);
+    article = yield ReaderMode.getArticleFromCache(url);
     if (article) {
       return article;
     }

mobile/android/base/tests/testReadingListCache.js

@@ -39,8 +39,7 @@ let TEST_PAGES = [
 ];
 
 add_task(function* test_article_not_found() {
-  let uri = Services.io.newURI(TEST_PAGES[0].url, null, null);
-  let article = yield ReaderMode.getArticleFromCache(uri);
+  let article = yield ReaderMode.getArticleFromCache(TEST_PAGES[0].url);
   do_check_eq(article, null);
 });
 
@@ -55,15 +54,13 @@ add_task(function* test_store_article() {
     length: TEST_PAGES[0].expected.length
   });
 
-  let uri = Services.io.newURI(TEST_PAGES[0].url, null, null);
-  let article = yield ReaderMode.getArticleFromCache(uri);
+  let article = yield ReaderMode.getArticleFromCache(TEST_PAGES[0].url);
   checkArticle(article, TEST_PAGES[0]);
 });
 
 add_task(function* test_remove_article() {
-  let uri = Services.io.newURI(TEST_PAGES[0].url, null, null);
-  yield ReaderMode.removeArticleFromCache(uri);
-  let article = yield ReaderMode.getArticleFromCache(uri);
+  yield ReaderMode.removeArticleFromCache(TEST_PAGES[0].url);
+  let article = yield ReaderMode.getArticleFromCache(TEST_PAGES[0].url);
   do_check_eq(article, null);
 });
 
@@ -110,8 +107,7 @@ add_task(function* test_migrate_cache() {
   yield Reader.migrateCache();
 
   // Check to make sure the article made it into the new cache.
-  let uri = Services.io.newURI(TEST_PAGES[0].url, null, null);
-  let article = yield ReaderMode.getArticleFromCache(uri);
+  let article = yield ReaderMode.getArticleFromCache(TEST_PAGES[0].url);
   checkArticle(article, TEST_PAGES[0]);
 });
 

mobile/android/chrome/content/Reader.js

@@ -28,8 +28,7 @@ let Reader = {
         break;
       }
       case "Reader:Removed": {
-        let uri = Services.io.newURI(aData, null, null);
-        ReaderMode.removeArticleFromCache(uri).catch(e => Cu.reportError("Error removing article from cache: " + e));
+        ReaderMode.removeArticleFromCache(aData).catch(e => Cu.reportError("Error removing article from cache: " + e));
 
         let mm = window.getGroupMessageManager("browsers");
         mm.broadcastAsyncMessage("Reader:Removed", { url: aData });
@@ -222,8 +221,8 @@ let Reader = {
       throw new Error("Can't add tab to reading list because no tab found for ID: " + tabID);
     }
 
-    let urlWithoutRef = tab.browser.currentURI.specIgnoringRef;
-    let article = yield this._getArticle(urlWithoutRef, tab.browser).catch(e => {
+    let url = tab.browser.currentURI.spec;
+    let article = yield this._getArticle(url, tab.browser).catch(e => {
       Cu.reportError("Error getting article for tab: " + e);
       return null;
     });
@@ -231,7 +230,7 @@ let Reader = {
       // If there was a problem getting the article, just store the
       // URL and title from the tab.
       article = {
-        url: urlWithoutRef,
+        url: url,
         title: tab.browser.contentDocument.title,
         length: 0,
         excerpt: "",
@@ -276,8 +275,7 @@ let Reader = {
     }
 
     // Next, try to find a parsed article in the cache.
-    let uri = Services.io.newURI(url, null, null);
-    article = yield ReaderMode.getArticleFromCache(uri);
+    article = yield ReaderMode.getArticleFromCache(url);
     if (article) {
       return article;
     }

mobile/android/chrome/content/content.js

@@ -73,8 +73,8 @@ let AboutReaderListener = {
 
           // The loaded page may have changed while we were parsing the document.
           // Make sure we've got the current one.
-          let currentURL = Services.io.newURI(content.document.documentURI, null, null).specIgnoringRef;
-          if (article.url !== currentURL) {
+          let url = Services.io.newURI(content.document.documentURI, null, null).spec;
+          if (article.url !== url) {
             return;
           }
 

toolkit/components/reader/ReaderMode.jsm

@@ -128,13 +128,13 @@ this.ReaderMode = {
   /**
    * Retrieves an article from the cache given an article URI.
    *
-   * @param uri The article URI.
+   * @param url The article URL.
    * @return {Promise}
    * @resolves JS object representing the article, or null if no article is found.
    * @rejects OS.File.Error
    */
-  getArticleFromCache: Task.async(function* (uri) {
-    let path = this._toHashedPath(uri.specIgnoringRef);
+  getArticleFromCache: Task.async(function* (url) {
+    let path = this._toHashedPath(url);
     try {
       let array = yield OS.File.read(path);
       return JSON.parse(new TextDecoder().decode(array));
@@ -161,13 +161,13 @@ this.ReaderMode = {
   /**
    * Removes an article from the cache given an article URI.
    *
-   * @param uri The article URI.
+   * @param url The article URL.
    * @return {Promise}
    * @resolves When the article is removed.
    * @rejects OS.File.Error
    */
-  removeArticleFromCache: Task.async(function* (uri) {
-    let path = this._toHashedPath(uri.specIgnoringRef);
+  removeArticleFromCache: Task.async(function* (url) {
+    let path = this._toHashedPath(url);
     yield OS.File.remove(path);
   }),
 
@@ -218,9 +218,10 @@ this.ReaderMode = {
           return;
         }
 
-        // Append URL to the article data. specIgnoringRef will ignore any hash
-        // in the URL.
-        article.url = uri.specIgnoringRef;
+        // Readability returns a URI object, but we only care about the URL.
+        article.url = article.uri.spec;
+        delete article.uri;
+
         let flags = Ci.nsIDocumentEncoder.OutputSelectionOnly | Ci.nsIDocumentEncoder.OutputAbsoluteLinks;
         article.title = Cc["@mozilla.org/parserutils;1"].getService(Ci.nsIParserUtils)
                                                         .convertToPlainText(article.title, flags, 0);