Bug 1246115 - Make gSafeBrowsing set the phishing menu item correctly. r?felipe draft
authorMike Conley <mconley@mozilla.com>
Wed, 10 Feb 2016 15:49:50 -0500
changeset 330163 66e0b464699099b2f1e41d78adb60aa327e9f10a
parent 329844 dc2c6c1a4fe3ed7413ce361e1dcf2c556d1384b0
child 514115 4af7ce4122b9ac0562d713c243d63cea188ea1f1
push id10694
push usermconley@mozilla.com
push dateWed, 10 Feb 2016 21:16:16 +0000
reviewersfelipe
bugs1246115
milestone47.0a1
Bug 1246115 - Make gSafeBrowsing set the phishing menu item correctly. r?felipe 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
browser/components/safebrowsing/content/test/browser.ini
toolkit/content/browser-child.js
toolkit/modules/RemoteWebProgress.jsm
--- a/browser/base/content/browser-safebrowsing.js
+++ b/browser/base/content/browser-safebrowsing.js
@@ -4,33 +4,35 @@
 
 // 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");
+    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;
 
+    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/browser/components/safebrowsing/content/test/browser.ini
+++ b/browser/components/safebrowsing/content/test/browser.ini
@@ -1,12 +1,10 @@
 [DEFAULT]
 support-files = head.js
 
 [browser_forbidden.js]
 [browser_bug400731.js]
 skip-if = e10s
 [browser_bug415846.js]
-skip-if = true
-# Disabled because it seems to now touch network resources
-# skip-if = os == "mac"
+skip-if = os == "mac"
 # Disabled on Mac because of its bizarre special-and-unique
 # snowflake of a help menu.
--- 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 about, 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;