Bug 1097906 - Be smarter about maintaining reader mode page action state. r=bnicholson

mobile/android/chrome/content/Reader.js

@@ -43,6 +43,10 @@ let Reader = {
   },
 
   updatePageAction: function(tab) {
+    if (!tab.getActive()) {
+      return;
+    }
+
     if (this.pageAction.id) {
       PageActions.remove(this.pageAction.id);
       delete this.pageAction.id;
@@ -65,7 +69,7 @@ let Reader = {
     // Only stop a reader session if the foreground viewer is not visible.
     UITelemetry.stopSession("reader.1", "", null);
 
-    if (tab.readerEnabled) {
+    if (tab.savedArticle) {
       this.pageAction.id = PageActions.add({
         title: Strings.browser.GetStringFromName("readerMode.enter"),
         icon: "drawable://reader",

mobile/android/chrome/content/browser.js

@@ -3149,8 +3149,6 @@ function Tab(aURL, aParams) {
   this._fixedMarginTop = 0;
   this._fixedMarginRight = 0;
   this._fixedMarginBottom = 0;
-  this._readerEnabled = false;
-  this._readerActive = false;
   this.userScrollPos = { x: 0, y: 0 };
   this.viewportExcludesHorizontalMargins = true;
   this.viewportExcludesVerticalMargins = true;
@@ -3907,6 +3905,8 @@ Tab.prototype = {
           if (contentDocument.body) {
             new AboutReader(contentDocument, this.browser.contentWindow);
           }
+          // Update the page action to show the "reader active" icon.
+          Reader.updatePageAction(this);
         }
 
         break;
@@ -4210,21 +4210,15 @@ Tab.prototype = {
           this.tilesData = null;
         }
 
-        if (!Reader.isEnabledForParseOnLoad) {
+        // Don't try to parse the document if reader mode is disabled,
+        // or if the page is already in reader mode.
+        if (!Reader.isEnabledForParseOnLoad || this.readerActive) {
           return;
         }
 
-        let resetReaderFlags = currentURL => {
-          // Don't clear the article for about:reader pages since we want to
-          // use the article from the previous page.
-          if (!currentURL.startsWith("about:reader")) {
-            this.savedArticle = null;
-            this.readerEnabled = false;
-            this.readerActive = false;
-          } else {
-            this.readerActive = true;
-          }
-        };
+        // Reader mode is disabled until proven enabled.
+        this.savedArticle = null;
+        Reader.updatePageAction(this);
 
         // Once document is fully loaded, parse it
         Reader.parseDocumentFromTab(this).then(article => {
@@ -4234,27 +4228,17 @@ Tab.prototype = {
 
           // Do nothing if there's no article or the page in this tab has changed.
           if (article == null || (article.url != currentURL)) {
-            resetReaderFlags(currentURL);
             return;
           }
 
           this.savedArticle = article;
+          Reader.updatePageAction(this);
 
           Messaging.sendRequest({
             type: "Content:ReaderEnabled",
             tabID: this.id
           });
-
-          if (this.readerActive) {
-            this.readerActive = false;
-          }
-          if (!this.readerEnabled) {
-            this.readerEnabled = true;
-          }
-        }).catch(e => {
-          Cu.reportError("Error parsing document from tab: " + e);
-          resetReaderFlags(this.browser.currentURI.specIgnoringRef);
-        });
+        }).catch(e => Cu.reportError("Error parsing document from tab: " + e));
       }
     }
   },
@@ -4799,24 +4783,8 @@ Tab.prototype = {
     }
   },
 
-  set readerEnabled(isReaderEnabled) {
-    this._readerEnabled = isReaderEnabled;
-    if (this.getActive())
-      Reader.updatePageAction(this);
-  },
-
-  get readerEnabled() {
-    return this._readerEnabled;
-  },
-
-  set readerActive(isReaderActive) {
-    this._readerActive = isReaderActive;
-    if (this.getActive())
-      Reader.updatePageAction(this);
-  },
-
   get readerActive() {
-    return this._readerActive;
+    return this.browser.currentURI.spec.startsWith("about:reader");
   },
 
   // nsIBrowserTab