Bug 1582751 - Show an error message when content blocking breakage reporting fails. r=nhnt11,fluent-reviewers,flod
☠☠ backed out by 0de7d040e6d1 ☠ ☠
authorJohann Hofmann <jhofmann@mozilla.com>
Sun, 03 Nov 2019 04:10:55 +0000
changeset 500293 be51af34cb7aa18cb3c8a23ad483bec686d97aa5
parent 500292 c753c25b8f7778ba839abcd50b2a05266886a7a8
child 500294 d08754485c250df14b1948ad31f03b8f1316b0aa
push id114164
push useraiakab@mozilla.com
push dateTue, 05 Nov 2019 10:06:15 +0000
treeherdermozilla-inbound@4d585c7edc76 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnhnt11, fluent-reviewers, flod
bugs1582751
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1582751 - Show an error message when content blocking breakage reporting fails. r=nhnt11,fluent-reviewers,flod Differential Revision: https://phabricator.services.mozilla.com/D51382
browser/base/content/browser-siteProtections.js
browser/base/content/browser.xhtml
browser/base/content/test/trackingUI/browser_trackingUI_report_breakage.js
browser/components/controlcenter/content/protectionsPanel.inc.xul
browser/locales/en-US/browser/protectionsPanel.ftl
browser/themes/shared/controlcenter/panel.inc.css
--- a/browser/base/content/browser-siteProtections.js
+++ b/browser/base/content/browser-siteProtections.js
@@ -1230,28 +1230,40 @@ var gProtectionsHandler = {
     ));
   },
   get _protectionsPopupSiteNotWorkingTPSwitch() {
     delete this._protectionsPopupSiteNotWorkingTPSwitch;
     return (this._protectionsPopupSiteNotWorkingTPSwitch = document.getElementById(
       "protections-popup-siteNotWorking-tp-switch"
     ));
   },
+  get _protectionsPopupSiteNotWorkingReportError() {
+    delete this._protectionsPopupSiteNotWorkingReportError;
+    return (this._protectionsPopupSiteNotWorkingReportError = document.getElementById(
+      "protections-popup-sendReportView-report-error"
+    ));
+  },
   get _protectionsPopupSendReportLearnMore() {
     delete this._protectionsPopupSendReportLearnMore;
     return (this._protectionsPopupSendReportLearnMore = document.getElementById(
       "protections-popup-sendReportView-learn-more"
     ));
   },
   get _protectionsPopupSendReportURL() {
     delete this._protectionsPopupSendReportURL;
     return (this._protectionsPopupSendReportURL = document.getElementById(
       "protections-popup-sendReportView-collection-url"
     ));
   },
+  get _protectionsPopupSendReportButton() {
+    delete this._protectionsPopupSendReportButton;
+    return (this._protectionsPopupSendReportButton = document.getElementById(
+      "protections-popup-sendReportView-submit"
+    ));
+  },
   get _trackingProtectionIconTooltipLabel() {
     delete this._trackingProtectionIconTooltipLabel;
     return (this._trackingProtectionIconTooltipLabel = document.getElementById(
       "tracking-protection-icon-tooltip-label"
     ));
   },
 
   get noTrackersDetectedDescription() {
@@ -2162,17 +2174,22 @@ var gProtectionsHandler = {
   showSendReportView() {
     // Save this URI to make sure that the user really only submits the location
     // they see in the report breakage dialog.
     this.reportURI = gBrowser.currentURI;
     let urlWithoutQuery = this.reportURI.asciiSpec.replace(
       "?" + this.reportURI.query,
       ""
     );
+    let commentsTextarea = document.getElementById(
+      "protections-popup-sendReportView-collection-comments"
+    );
+    commentsTextarea.value = "";
     this._protectionsPopupSendReportURL.value = urlWithoutQuery;
+    this._protectionsPopupSiteNotWorkingReportError.hidden = true;
     this._protectionsPopupMultiView.showSubView(
       "protections-popup-sendReportView"
     );
   },
 
   toggleBreakageLink() {
     // The breakage link will only be shown if tracking protection is enabled
     // for the site and the TP toggle state is on. And we won't show the
@@ -2258,38 +2275,40 @@ var gProtectionsHandler = {
         activatedBlockers.push(blocker.reportBreakageLabel);
       }
     }
 
     if (activatedBlockers.length) {
       formData.set("labels", activatedBlockers.join(","));
     }
 
+    this._protectionsPopupSendReportButton.disabled = true;
+
     fetch(reportEndpoint, {
       method: "POST",
       credentials: "omit",
       body: formData,
     })
-      .then(function(response) {
+      .then(response => {
+        this._protectionsPopupSendReportButton.disabled = false;
         if (!response.ok) {
           Cu.reportError(
             `Content Blocking report to ${reportEndpoint} failed with status ${
               response.status
             }`
           );
+          this._protectionsPopupSiteNotWorkingReportError.hidden = false;
         } else {
-          // Clear the textarea value when the report is submitted
-          commentsTextarea.value = "";
+          this._protectionsPopup.hidePopup();
         }
       })
       .catch(Cu.reportError);
   },
 
   onSendReportClicked() {
-    this._protectionsPopup.hidePopup();
     this.submitBreakageReport(this.reportURI);
   },
 
   async maybeUpdateEarliestRecordedDateTooltip() {
     if (this._hasEarliestRecord) {
       return;
     }
 
--- a/browser/base/content/browser.xhtml
+++ b/browser/base/content/browser.xhtml
@@ -72,16 +72,17 @@
         persist="screenX screenY width height sizemode"
         data-l10n-sync="true">
 
 <linkset>
   <html:link rel="localization" href="branding/brand.ftl"/>
   <html:link rel="localization" href="browser/branding/sync-brand.ftl"/>
   <html:link rel="localization" href="browser/browser.ftl"/>
   <html:link rel="localization" href="browser/menubar.ftl"/>
+  <html:link rel="localization" href="browser/protectionsPanel.ftl"/>
   <html:link rel="localization" href="browser/appmenu.ftl"/>
   <html:link rel="localization" href="browser/readerView.ftl"/>
 </linkset>
 
 # All JS files which are needed by browser.xhtml and other top level windows to
 # support MacOS specific features *must* go into the global-scripts.inc file so
 # that they can be shared with macWindow.inc.xul.
 #include global-scripts.inc
--- a/browser/base/content/test/trackingUI/browser_trackingUI_report_breakage.js
+++ b/browser/base/content/test/trackingUI/browser_trackingUI_report_breakage.js
@@ -117,16 +117,27 @@ add_task(async function testNoTracking()
     );
     ok(
       BrowserTestUtils.is_hidden(siteNotWorkingButton),
       "site not working button is not visible"
     );
   });
 });
 
+add_task(async function testReportBreakageError() {
+  Services.prefs.setBoolPref(TP_PREF, true);
+  // Make sure that we correctly strip the query.
+  let url = TRACKING_PAGE + "?a=b&1=abc&unicode=🦊";
+  await BrowserTestUtils.withNewTab(url, async function() {
+    await testReportBreakage(TRACKING_PAGE, "trackingprotection", true);
+  });
+
+  Services.prefs.clearUserPref(TP_PREF);
+});
+
 add_task(async function testTP() {
   Services.prefs.setBoolPref(TP_PREF, true);
   // Make sure that we correctly strip the query.
   let url = TRACKING_PAGE + "?a=b&1=abc&unicode=🦊";
   await BrowserTestUtils.withNewTab(url, async function() {
     await testReportBreakage(TRACKING_PAGE, "trackingprotection");
   });
 
@@ -180,17 +191,17 @@ add_task(async function testCM() {
 
     await testReportBreakage(TRACKING_PAGE, "cryptomining");
   });
 
   Services.prefs.clearUserPref(CM_PREF);
   Services.prefs.clearUserPref(CB_PREF);
 });
 
-async function testReportBreakage(url, tags) {
+async function testReportBreakage(url, tags, error = false) {
   // Setup a mock server for receiving breakage reports.
   let server = new HttpServer();
   server.start(-1);
   let i = server.identity;
   let path =
     i.primaryScheme + "://" + i.primaryHost + ":" + i.primaryPort + "/";
 
   Services.prefs.setStringPref(PREF_REPORT_BREAKAGE_URL, path);
@@ -294,22 +305,45 @@ async function testReportBreakage(url, t
             "This is a comment\r\n",
           'Content-Disposition: form-data; name="labels"\r\n\r\n' +
             `${tags}\r\n`,
           "",
         ],
         "Should send the correct form data"
       );
 
+      if (error) {
+        response.setStatusLine(request.httpVersion, 500, "Request failed");
+      } else {
+        response.setStatusLine(request.httpVersion, 201, "Entry created");
+      }
+
       resolve();
     });
 
     comments.value = "This is a comment";
     submitButton.click();
   });
 
+  let errorMessage = document.getElementById(
+    "protections-popup-sendReportView-report-error"
+  );
+  if (error) {
+    await BrowserTestUtils.waitForCondition(() =>
+      BrowserTestUtils.is_visible(errorMessage)
+    );
+    is(
+      comments.value,
+      "This is a comment",
+      "Comment not cleared in case of an error"
+    );
+    gProtectionsHandler._protectionsPopup.hidePopup();
+  } else {
+    ok(BrowserTestUtils.is_hidden(errorMessage), "Error message not shown");
+  }
+
   await popuphidden;
 
   // Stop the server.
   await new Promise(r => server.stop(r));
 
   Services.prefs.clearUserPref(PREF_REPORT_BREAKAGE_URL);
 }
--- a/browser/components/controlcenter/content/protectionsPanel.inc.xul
+++ b/browser/components/controlcenter/content/protectionsPanel.inc.xul
@@ -305,16 +305,19 @@
           <vbox class="protections-popup-sendReportView-collection-section">
             <label control="protections-popup-sendReportView-collection-url">&contentBlocking.breakageReportView.collection.url.label;</label>
             <html:input readonly="readonly" id="protections-popup-sendReportView-collection-url" aria-label="&contentBlocking.breakageReportView.collection.url.label;"/>
           </vbox>
           <vbox class="protections-popup-sendReportView-collection-section">
             <label control="protections-popup-sendReportView-collection-comments">&contentBlocking.breakageReportView2.collection.comments.label;</label>
             <html:textarea id="protections-popup-sendReportView-collection-comments" aria-label="&contentBlocking.breakageReportView2.collection.comments.label;"/>
           </vbox>
+          <label id="protections-popup-sendReportView-report-error"
+            data-l10n-id="protections-panel-sendreportview-error"
+            hidden="true" role="alert"></label>
         </vbox>
         <vbox id="protections-popup-sendReportView-footer"
               class="panel-footer">
           <button id="protections-popup-sendReportView-cancel"
                   label="&contentBlocking.breakageReportView.cancel.label;"
                   oncommand="gProtectionsHandler._protectionsPopupMultiView.goBack();"/>
           <button id="protections-popup-sendReportView-submit"
                   default="true"
new file mode 100644
--- /dev/null
+++ b/browser/locales/en-US/browser/protectionsPanel.ftl
@@ -0,0 +1,5 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+protections-panel-sendreportview-error = There was an error sending the report. Please try again later.
--- a/browser/themes/shared/controlcenter/panel.inc.css
+++ b/browser/themes/shared/controlcenter/panel.inc.css
@@ -474,16 +474,21 @@ description#identity-popup-content-verif
 #protections-popup-sendReportView-collection-comments {
   height: 120px;
 }
 
 #protections-popup-sendReportView label {
   margin-inline-start: 0;
 }
 
+#protections-popup-sendReportView-report-error {
+  margin-bottom: 24px;
+  color: #d74345;
+}
+
 .protections-popup-category {
   /* Overwrite toolbarbutton styles */
   -moz-appearance: none;
   margin: 0;
   padding-inline-start: 0;
 }
 
 .protections-popup-category:-moz-focusring,