Bug 1144680 - The Reading List URLbar button should handle about:reader urls and filter out other non-http(s) urls, r=markh.
authorFlorian Quèze <florian@queze.net>
Thu, 19 Mar 2015 15:50:23 -0700
changeset 263429 fafaf621ada2f629c7c689fcdbe8d53ddb02b577
parent 263428 89cb4fd9ee1db6ac37d81adf8394a042c34d7a61
child 263430 cc408bbacbd940ca014f00aa70644c2e3aff4d29
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs1144680
milestone39.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1144680 - The Reading List URLbar button should handle about:reader urls and filter out other non-http(s) urls, r=markh.
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;
     }