Bug 1182778, r=margaret,f=bz

browser/app/profile/firefox.js

@@ -1929,6 +1929,10 @@ pref("browser.reader.detectedFirstArticle", false);
 // Don't limit how many nodes we care about on desktop:
 pref("reader.parse-node-limit", 0);
 
+// On desktop, we want the URLs to be included here for ease of debugging,
+// and because (normally) these errors are not persisted anywhere.
+pref("reader.errors.includeURLs", true);
+
 pref("browser.pocket.enabled", true);
 pref("browser.pocket.api", "api.getpocket.com");
 pref("browser.pocket.site", "getpocket.com");

browser/modules/ReaderParent.jsm

@@ -71,6 +71,10 @@ let ReaderParent = {
           if (message.target.messageManager) {
             message.target.messageManager.sendAsyncMessage("Reader:ArticleData", { article: article });
           }
+        }, e => {
+          if (e && e.newURL) {
+            message.target.loadURI("about:reader?url=" + encodeURIComponent(e.newURL));
+          }
         });
         break;
 
@@ -230,6 +234,10 @@ let ReaderParent = {
    */
   _getArticle: Task.async(function* (url, browser) {
     return yield ReaderMode.downloadAndParseDocument(url).catch(e => {
+      if (e && e.newURL) {
+        // Pass up the error so we can navigate the browser in question to the new URL:
+        throw e;
+      }
       Cu.reportError("Error downloading and parsing document: " + e);
       return null;
     });

mobile/android/chrome/content/Reader.js

@@ -59,6 +59,10 @@ let Reader = {
           if (message.target.messageManager) {
             message.target.messageManager.sendAsyncMessage("Reader:ArticleData", { article: article });
           }
+        }, e => {
+          if (e && e.newURL) {
+            message.target.loadURI("about:reader?url=" + encodeURIComponent(e.newURL));
+          }
         });
         break;
 
@@ -289,6 +293,10 @@ let Reader = {
     // Article hasn't been found in the cache, we need to
     // download the page and parse the article out of it.
     return yield ReaderMode.downloadAndParseDocument(url).catch(e => {
+      if (e && e.newURL) {
+        // Pass up the error so we can navigate the browser in question to the new URL:
+        throw e;
+      }
       Cu.reportError("Error downloading and parsing document: " + e);
       return null;
     });

modules/libpref/init/all.js

@@ -5008,6 +5008,10 @@ pref("reader.parse-node-limit", 3000);
 // is disabled by default.
 pref("reader.parse-on-load.force-enabled", false);
 
+// Whether we include full URLs in browser console errors. This is disabled
+// by default because some platforms will persist these, leading to privacy issues.
+pref("reader.errors.includeURLs", false);
+
 // The default relative font size in reader mode (1-9)
 pref("reader.font_size", 5);
 

toolkit/components/reader/AboutReader.jsm

@@ -23,7 +23,10 @@ let gStrings = Services.strings.createBundle("chrome://global/locale/aboutReader
 let AboutReader = function(mm, win, articlePromise) {
   let url = this._getOriginalUrl(win);
   if (!(url.startsWith("http://") || url.startsWith("https://"))) {
-    Cu.reportError("Only http:// and https:// URLs can be loaded in about:reader");
+    let errorMsg = "Only http:// and https:// URLs can be loaded in about:reader.";
+    if (Services.prefs.getBoolPref("reader.errors.includeURLs"))
+      errorMsg += " Tried to load: " + url + ".";
+    Cu.reportError(errorMsg);
     win.location.href = "about:blank";
     return;
   }

toolkit/components/reader/ReaderMode.jsm

@@ -209,7 +209,21 @@ this.ReaderMode = {
             let urlIndex = content.toUpperCase().indexOf("URL=");
             if (urlIndex > -1) {
               let url = content.substring(urlIndex + 4);
-              this._downloadDocument(url).then((doc) => resolve(doc));
+              let ssm = Services.scriptSecurityManager;
+              let flags = ssm.LOAD_IS_AUTOMATIC_DOCUMENT_REPLACEMENT |
+                          ssm.DISALLOW_INHERIT_PRINCIPAL;
+              try {
+                ssm.checkLoadURIStrWithPrincipal(doc.nodePrincipal, url, flags);
+              } catch (ex) {
+                let errorMsg = "Reader mode disallowed meta refresh (reason: " + ex + ").";
+
+                if (Services.prefs.getBoolPref("reader.errors.includeURLs"))
+                  errorMsg += " Refresh target URI: '" + url + "'.";
+                reject(errorMsg);
+                return;
+              }
+              // Otherwise, pass an object indicating our new URL:
+              reject({newURL: url});
               return;
             }
           }