Bug 1144680 - The Reading List URLbar button should handle about:reader urls and filter out other non-http(s) urls, r=markh. a=readinglist
authorFlorian Quèze <florian@queze.net>
Thu, 19 Mar 2015 15:50:23 -0700
changeset 248440 70a942a84ad58abe0963b800c186deb5a7fe8f6c
parent 248439 69c5f5536e10e7119f277e93e27695eba3107ed0
child 248441 73a88d5a05d3ced5caf3b51c85773defe86eacc1
push id7837
push userjwein@mozilla.com
push dateFri, 27 Mar 2015 00:27:16 +0000
treeherdermozilla-aurora@cb0db44ce60e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh, readinglist
bugs1144680
milestone38.0a2
Bug 1144680 - The Reading List URLbar button should handle about:reader urls and filter out other non-http(s) urls, r=markh. a=readinglist
browser/base/content/browser-readinglist.js
browser/components/readinglist/ReadingList.jsm
--- a/browser/base/content/browser-readinglist.js
+++ b/browser/base/content/browser-readinglist.js
@@ -227,22 +227,32 @@ let ReadingListUI = {
    *
    * @param {string} state - New state. Either "valid" or "invalid".
    */
   onPageProxyStateChanged: Task.async(function* (state) {
     if (!this.toolbarButton) {
       // nothing to do if we have no button.
       return;
     }
-    if (!this.enabled || state == "invalid") {
+
+    let uri;
+    if (this.enabled && state == "valid") {
+      uri = gBrowser.currentURI;
+      if (uri.schemeIs("about"))
+        uri = ReaderParent.parseReaderUrl(uri.spec);
+      else if (!uri.schemeIs("http") && !uri.schemeIs("https"))
+        uri = null;
+    }
+
+    if (!uri) {
       this.toolbarButton.setAttribute("hidden", true);
       return;
     }
 
-    let isInList = yield ReadingList.containsURL(gBrowser.currentURI);
+    let isInList = yield ReadingList.containsURL(uri);
     this.setToolbarButtonState(isInList);
   }),
 
   /**
    * Set the state of the ReadingList toolbar button in the urlbar.
    * If the current tab's page is in the ReadingList (active), sets the button
    * to allow removing the page. Otherwise, sets the button to allow adding the
    * page (not active).
@@ -263,32 +273,41 @@ let ReadingListUI = {
   /**
    * Toggle a page (from a browser) in the ReadingList, adding if it's not already added, or
    * removing otherwise.
    *
    * @param {<xul:browser>} browser - Browser with page to toggle.
    * @returns {Promise} Promise resolved when operation has completed.
    */
   togglePageByBrowser: Task.async(function* (browser) {
-    let item = yield ReadingList.getItemForURL(browser.currentURI);
+    let uri = browser.currentURI;
+    if (uri.spec.startsWith("about:reader?"))
+      uri = ReaderParent.parseReaderUrl(uri.spec);
+    if (!uri)
+      return;
+
+    let item = yield ReadingList.getItemForURL(uri);
     if (item) {
       yield item.delete();
     } else {
-      yield ReadingList.addItemFromBrowser(browser);
+      yield ReadingList.addItemFromBrowser(browser, uri);
     }
   }),
 
   /**
    * Checks if a given item matches the current tab in this window.
    *
    * @param {ReadingListItem} item - Item to check
    * @returns True if match, false otherwise.
    */
   isItemForCurrentBrowser(item) {
     let currentURL = gBrowser.currentURI.spec;
+    if (currentURL.startsWith("about:reader?"))
+      currentURL = ReaderParent.parseReaderUrl(currentURL);
+
     if (item.url == currentURL || item.resolvedURL == currentURL) {
       return true;
     }
     return false;
   },
 
   /**
    * ReadingList event handler for when an item is added.
--- a/browser/components/readinglist/ReadingList.jsm
+++ b/browser/components/readinglist/ReadingList.jsm
@@ -284,23 +284,25 @@ ReadingListImpl.prototype = {
     let url = normalizeURI(uri).spec;
     let [item] = yield this.iterator({url: url}, {resolvedURL: url}).items(1);
     return item;
   }),
 
    /**
    * Add to the ReadingList the page that is loaded in a given browser.
    *
-   * @param {<xul:browser>} browser - Browser element for the document.
+   * @param {<xul:browser>} browser - Browser element for the document,
+   * used to get metadata about the article.
+   * @param {nsIURI/string} url - url to add to the reading list.
    * @return {Promise} Promise that is fullfilled with the added item.
    */
-  addItemFromBrowser: Task.async(function* (browser) {
+  addItemFromBrowser: Task.async(function* (browser, url) {
     let metadata = yield getMetadataFromBrowser(browser);
     let itemData = {
-      url: browser.currentURI,
+      url: url,
       title: metadata.title,
       resolvedURL: metadata.url,
       excerpt: metadata.description,
     };
 
     if (metadata.description) {
       itemData.exerpt = metadata.description;
     }