Bug 1246115 - Make gSafeBrowsing set the phishing menu item correctly. r=Gijs, a=lizzard
authorMike Conley <mconley@mozilla.com>
Wed, 10 Feb 2016 15:49:50 -0500
changeset 318939 d02f6fb2a68c008a0c2a36cb6263ac54cef6a83d
parent 318938 7a7c3d89e43b2d8b6f6e367c898cd621bec43c78
child 318940 9b4821cbe633477fa8557e4baf45fc089522b5ba
push id1079
push userjlund@mozilla.com
push dateFri, 15 Apr 2016 21:02:33 +0000
treeherdermozilla-release@575fbf6786d5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs, lizzard
bugs1246115
milestone46.0a2
Bug 1246115 - Make gSafeBrowsing set the phishing menu item correctly. r=Gijs, a=lizzard Unfortunately, when onLocationChange is fired for an attack site for the about:blocked error page that we display, content.document has not been updated with the loaded error document, so content.document.documentURI will appear to be the previous page that had been loaded. In this patch, we update the parent's cache of documentURI in onStateChange as well, since this seems to be fired after the error page has been loaded. MozReview-Commit-ID: 1yLAw0JTEC6
browser/base/content/browser-safebrowsing.js
toolkit/content/browser-child.js
toolkit/modules/RemoteWebProgress.jsm
--- a/browser/base/content/browser-safebrowsing.js
+++ b/browser/base/content/browser-safebrowsing.js
@@ -3,34 +3,42 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 // Note: this file is not shipped (through jar.mn)
 // if MOZ_SAFE_BROWSING is not defined.
 
 var gSafeBrowsing = {
 
   setReportPhishingMenu: function() {
-    // A phishing page will have a specific about:blocked content documentURI
-    var uri = gBrowser.currentURI;
-    var isPhishingPage = uri && uri.spec.startsWith("about:blocked?e=phishingBlocked");
+    // In order to detect whether or not we're at the phishing warning
+    // page, we have to check the documentURI instead of the currentURI.
+    // This is because when the DocShell loads an error page, the
+    // currentURI stays at the original target, while the documentURI
+    // will point to the internal error page we loaded instead.
+    var docURI = gBrowser.selectedBrowser.documentURI;
+    var isPhishingPage =
+      docURI && docURI.spec.startsWith("about:blocked?e=phishingBlocked");
 
     // Show/hide the appropriate menu item.
     document.getElementById("menu_HelpPopup_reportPhishingtoolmenu")
             .hidden = isPhishingPage;
     document.getElementById("menu_HelpPopup_reportPhishingErrortoolmenu")
             .hidden = !isPhishingPage;
 
     var broadcasterId = isPhishingPage
                         ? "reportPhishingErrorBroadcaster"
                         : "reportPhishingBroadcaster";
 
     var broadcaster = document.getElementById(broadcasterId);
     if (!broadcaster)
       return;
 
+    // Now look at the currentURI to learn which page we were trying
+    // to browse to.
+    let uri = gBrowser.currentURI;
     if (uri && (uri.schemeIs("http") || uri.schemeIs("https")))
       broadcaster.removeAttribute("disabled");
     else
       broadcaster.setAttribute("disabled", true);
   },
 
   /**
    * Used to report a phishing page or a false positive
--- a/toolkit/content/browser-child.js
+++ b/toolkit/content/browser-child.js
@@ -117,16 +117,24 @@ var WebProgressListener = {
 
   onStateChange: function onStateChange(aWebProgress, aRequest, aStateFlags, aStatus) {
     let json = this._setupJSON(aWebProgress, aRequest);
     let objects = this._setupObjects(aWebProgress, aRequest);
 
     json.stateFlags = aStateFlags;
     json.status = aStatus;
 
+    // It's possible that this state change was triggered by
+    // loading an internal error page, for which the parent
+    // will want to know some details, so we'll update it with
+    // the documentURI.
+    if (aWebProgress && aWebProgress.isTopLevel) {
+      json.documentURI = content.document.documentURIObject.spec;
+    }
+
     this._send("Content:StateChange", json, objects);
   },
 
   onProgressChange: function onProgressChange(aWebProgress, aRequest, aCurSelf, aMaxSelf, aCurTotal, aMaxTotal) {
     let json = this._setupJSON(aWebProgress, aRequest);
     let objects = this._setupObjects(aWebProgress, aRequest);
 
     json.curSelf = aCurSelf;
--- a/toolkit/modules/RemoteWebProgress.jsm
+++ b/toolkit/modules/RemoteWebProgress.jsm
@@ -210,16 +210,19 @@ RemoteWebProgressManager.prototype = {
 
     if (isTopLevel) {
       this._browser._contentWindow = objects.contentWindow;
       this._browser._documentContentType = json.documentContentType;
     }
 
     switch (aMessage.name) {
     case "Content:StateChange":
+      if (isTopLevel) {
+        this._browser._documentURI = newURI(json.documentURI);
+      }
       this._callProgressListeners("onStateChange", webProgress, request, json.stateFlags, json.status);
       break;
 
     case "Content:LocationChange":
       let location = newURI(json.location);
       let flags = json.flags;
       let remoteWebNav = this._browser._remoteWebNavigationImpl;