Bug 1152219 - Make reader mode node limit a pref, turn off entirely for desktop because of isProbablyReaderable. r=margaret, a=sledru
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Thu, 16 Apr 2015 16:24:08 +0100
changeset 258531 4a98323f8e68
parent 258530 e5d6dc48f6de
child 258532 849bf3c58408
push id4690
push userryanvm@gmail.com
push date2015-04-20 16:04 +0000
treeherdermozilla-beta@eb5e2063637b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmargaret, sledru
bugs1152219
milestone38.0
Bug 1152219 - Make reader mode node limit a pref, turn off entirely for desktop because of isProbablyReaderable. r=margaret, a=sledru
browser/app/profile/firefox.js
modules/libpref/init/all.js
toolkit/components/reader/ReaderMode.jsm
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -1883,11 +1883,15 @@ pref("dom.ipc.processHangMonitor", true)
 pref("dom.ipc.reportProcessHangs", false);
 #else
 pref("dom.ipc.reportProcessHangs", true);
 #endif
 
 // Enable ReadingList browser UI by default.
 pref("browser.readinglist.enabled", true);
 pref("browser.readinglist.sidebarEverOpened", false);
+
 // Enable the readinglist engine by default.
 pref("readinglist.scheduler.enabled", true);
 pref("readinglist.server", "https://readinglist.services.mozilla.com/v1");
+
+// Don't limit how many nodes we care about on desktop:
+pref("reader.parse-node-limit", 0);
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -4521,16 +4521,20 @@ pref("media.gmp-manager.certs.1.commonNa
 pref("media.gmp-manager.certs.2.issuerName", "CN=Thawte SSL CA,O=\"Thawte, Inc.\",C=US");
 pref("media.gmp-manager.certs.2.commonName", "aus4.mozilla.org");
 #endif
 
 // Whether or not to perform reader mode article parsing on page load.
 // If this pref is disabled, we will never show a reader mode icon in the toolbar.
 pref("reader.parse-on-load.enabled", true);
 
+// After what size document we don't bother running Readability on it
+// because it'd slow things down too much
+pref("reader.parse-node-limit", 3000);
+
 // Force-enables reader mode parsing, even on low-memory platforms, where it
 // is disabled by default.
 pref("reader.parse-on-load.force-enabled", false);
 
 // The default relative font size in reader mode (1-9)
 pref("reader.font_size", 5);
 
 // The default color scheme in reader mode (light, dark, sepia, auto)
--- a/toolkit/components/reader/ReaderMode.jsm
+++ b/toolkit/components/reader/ReaderMode.jsm
@@ -27,17 +27,22 @@ XPCOMUtils.defineLazyGetter(this, "Reada
 this.ReaderMode = {
   // Version of the cache schema.
   CACHE_VERSION: 1,
 
   DEBUG: 0,
 
   // Don't try to parse the page if it has too many elements (for memory and
   // performance reasons)
-  MAX_ELEMS_TO_PARSE: 3000,
+  get maxElemsToParse() {
+    delete this.parseNodeLimit;
+
+    Services.prefs.addObserver("reader.parse-node-limit", this, false);
+    return this.parseNodeLimit = Services.prefs.getIntPref("reader.parse-node-limit");
+  },
 
   get isEnabledForParseOnLoad() {
     delete this.isEnabledForParseOnLoad;
 
     // Listen for future pref changes.
     Services.prefs.addObserver("reader.parse-on-load.", this, false);
 
     return this.isEnabledForParseOnLoad = this._getStateForParseOnLoad();
@@ -57,16 +62,18 @@ this.ReaderMode = {
     return isForceEnabled || (isEnabled && !this.isOnLowMemoryPlatform);
   },
 
   observe: function(aMessage, aTopic, aData) {
     switch(aTopic) {
       case "nsPref:changed":
         if (aData.startsWith("reader.parse-on-load.")) {
           this.isEnabledForParseOnLoad = this._getStateForParseOnLoad();
+        } else if (aData === "reader.parse-node-limit") {
+          this.parseNodeLimit = Services.prefs.getIntPref(aData);
         }
         break;
     }
   },
 
   /**
    * Returns original URL from an about:reader URL.
    *
@@ -243,20 +250,22 @@ this.ReaderMode = {
    * in readerWorker.js.
    *
    * @param uri The article URI.
    * @param doc The document to parse.
    * @return {Promise}
    * @resolves JS object representing the article, or null if no article is found.
    */
   _readerParse: Task.async(function* (uri, doc) {
-    let numTags = doc.getElementsByTagName("*").length;
-    if (numTags > this.MAX_ELEMS_TO_PARSE) {
-      this.log("Aborting parse for " + uri.spec + "; " + numTags + " elements found");
-      return null;
+    if (this.parseNodeLimit) {
+      let numTags = doc.getElementsByTagName("*").length;
+      if (numTags > this.parseNodeLimit) {
+        this.log("Aborting parse for " + uri.spec + "; " + numTags + " elements found");
+        return null;
+      }
     }
 
     let uriParam = {
       spec: uri.spec,
       host: uri.host,
       prePath: uri.prePath,
       scheme: uri.scheme,
       pathBase: Services.io.newURI(".", null, uri).spec