Merge pull request #606 from Ascrod/master

Updates to Reader and Narrator components.

2 files changed, 16 insertions, 38 deletions

diff --git a/toolkit/components/narrate/NarrateControls.jsm b/toolkit/components/narrate/NarrateControls.jsm
index be3ce636c..56b3deaf8 100644
--- a/toolkit/components/narrate/NarrateControls.jsm
+++ b/toolkit/components/narrate/NarrateControls.jsm
@@ -10,7 +10,6 @@ Cu.import("resource://gre/modules/narrate/VoiceSelect.jsm");
 Cu.import("resource://gre/modules/narrate/Narrator.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/AsyncPrefs.jsm");
-Cu.import("resource://gre/modules/TelemetryStopwatch.jsm");
 
 this.EXPORTED_SYMBOLS = ["NarrateControls"];
 
@@ -147,11 +146,6 @@ NarrateControls.prototype = {
       case "voiceschanged":
         this._setupVoices();
         break;
-      case "unload":
-        if (this.narrator.speaking) {
-          TelemetryStopwatch.finish("NARRATE_CONTENT_SPEAKTIME_MS", this);
-        }
-        break;
     }
   },
 
@@ -187,20 +181,9 @@ NarrateControls.prototype = {
       }
 
       let narrateToggle = win.document.querySelector(".narrate-toggle");
-      let histogram = Services.telemetry.getKeyedHistogramById(
-        "NARRATE_CONTENT_BY_LANGUAGE_2");
       let initial = !this._voicesInitialized;
       this._voicesInitialized = true;
 
-      if (initial) {
-        histogram.add(language, 0);
-      }
-
-      if (options.length && narrateToggle.hidden) {
-        // About to show for the first time..
-        histogram.add(language, 1);
-      }
-
       // We disable this entire feature if there are no available voices.
       narrateToggle.hidden = !options.length;
     });
@@ -265,12 +248,6 @@ NarrateControls.prototype = {
 
     this._doc.querySelector(".narrate-skip-previous").disabled = !speaking;
     this._doc.querySelector(".narrate-skip-next").disabled = !speaking;
-
-    if (speaking) {
-      TelemetryStopwatch.start("NARRATE_CONTENT_SPEAKTIME_MS", this);
-    } else {
-      TelemetryStopwatch.finish("NARRATE_CONTENT_SPEAKTIME_MS", this);
-    }
   },
 
   _createVoiceLabel(voice) {
diff --git a/toolkit/components/reader/AboutReader.jsm b/toolkit/components/reader/AboutReader.jsm
index fb82e5789..c5d04476d 100644
--- a/toolkit/components/reader/AboutReader.jsm
+++ b/toolkit/components/reader/AboutReader.jsm
@@ -200,9 +200,6 @@ AboutReader.prototype = {
         } else if (!target.closest(".dropdown-popup")) {
           this._closeDropdowns();
         }
-        if (target.tagName == "A" && !target.classList.contains("reader-domain")) {
-          this._linkClicked(aEvent);
-        }
         break;
       case "scroll":
         this._closeDropdowns(true);
@@ -720,6 +717,21 @@ AboutReader.prototype = {
     }
   },
 
+  _fixLocalLinks() {
+    // We need to do this because preprocessing the content through nsIParserUtils
+    // gives back a DOM with a <base> element. That influences how these URLs get
+    // resolved, making them no longer match the document URI (which is
+    // about:reader?url=...). To fix this, make all the hash URIs absolute. This
+    // is hacky, but the alternative of removing the base element has potential
+    // security implications if Readability has not successfully made all the URLs
+    // absolute, so we pick just fixing these in-document links explicitly.
+    let localLinks = this._contentElement.querySelectorAll("a[href^='#']");
+    for (let localLink of localLinks) {
+      // Have to get the attribute because .href provides an absolute URI.
+      localLink.href = this._doc.documentURI + localLink.getAttribute("href");
+    }
+  },
+
   _formatReadTime(slowEstimate, fastEstimate) {
     let displayStringKey = "aboutReader.estimatedReadTimeRange1";
 
@@ -790,6 +802,7 @@ AboutReader.prototype = {
       false, articleUri, this._contentElement);
     this._contentElement.innerHTML = "";
     this._contentElement.appendChild(contentFragment);
+    this._fixLocalLinks();
     this._maybeSetTextDirection(article);
     this._foundLanguage(article.language);
 
@@ -978,18 +991,6 @@ AboutReader.prototype = {
   },
 
   /*
-   * Override link handling for same-page references so we don't exit Reader View.
-   */
-  _linkClicked(event) {
-    var originalUrl = Services.io.newURI(this._getOriginalUrl(), null, null);
-    var targetUrl = Services.io.newURI(event.target.href, null, null);
-    if (originalUrl.specIgnoringRef == targetUrl.specIgnoringRef) {
-      event.preventDefault();
-      this._goToReference(targetUrl.ref);
-    }
-  },
-
-  /*
    * Scroll reader view to a reference
    */
   _goToReference(ref) {