bug 866304 DialogUI "modals" cleanup patch. Removes the "modals" portions of DialogUI. Converts the crash reporter prompt to use nsIPromptService.confirmEx instead of DialogUI.importModal. r=mbrubeck
authorTim Abraldes <tabraldes@mozilla.com>
Mon, 05 Aug 2013 23:29:55 -0700
changeset 154570 daf88d34960c94fcbd77e9eff76db9d2c4b7b1bb
parent 154569 4716cc685a06df51ba7b703154d323f41d5a840b
child 154571 a0b47189f0a60f5bf189fc1332cd325fbe652f6f
push id2961
push userlsblakk@mozilla.com
push dateMon, 28 Oct 2013 21:59:28 +0000
treeherdermozilla-beta@73ef4f13486f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmbrubeck
bugs866304
milestone26.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 866304 DialogUI "modals" cleanup patch. Removes the "modals" portions of DialogUI. Converts the crash reporter prompt to use nsIPromptService.confirmEx instead of DialogUI.importModal. r=mbrubeck The crash reporter prompt is the only consumer of DialogUI.importModal and related code. The "modals" concept that DialogUI implements should go away in favor of tab-modal dialogs and tab-agnostic notifications (e.g. flyouts). I don't think that the crash reporter prompt should be a tab modal dialog; I'd prefer to see it become an about: page or perhaps a flyout. Making it a tab-modal prompt was just the easiest thing to do in this set of patches.
browser/metro/base/content/browser-ui.js
browser/metro/base/tests/mochitest/browser_crashprompt.js
browser/metro/locales/jar.mn
--- a/browser/metro/base/content/browser-ui.js
+++ b/browser/metro/base/content/browser-ui.js
@@ -253,33 +253,59 @@ var BrowserUI = {
   },
 
   startupCrashCheck: function startupCrashCheck() {
 #ifdef MOZ_CRASHREPORTER
     if (!CrashReporter.enabled) {
       return;
     }
     let lastCrashID = this.lastCrashID;
+
     if (!lastCrashID || !lastCrashID.length) {
       return;
     }
+
     let shouldReport = Services.prefs.getBoolPref("app.crashreporter.autosubmit");
     let didPrompt = Services.prefs.getBoolPref("app.crashreporter.prompted");
 
     if (!shouldReport && !didPrompt) {
-      // We have a crash to submit, we haven't prompted for approval yet,
-      // and the auto-submit pref is false, prompt. The dialog will call
-      // startupCrashCheck again if the user approves.
+      let crashBundle = Services.strings.createBundle("chrome://browser/locale/crashprompt.properties");
+      let title = crashBundle.GetStringFromName("crashprompt.dialog.title");
+      let acceptbutton = crashBundle.GetStringFromName("crashprompt.dialog.acceptbutton");
+      let refusebutton = crashBundle.GetStringFromName("crashprompt.dialog.refusebutton");
+      let bodyText = crashBundle.GetStringFromName("crashprompt.dialog.statement1");
+
+      let buttonPressed =
+            Services.prompt.confirmEx(
+                null,
+                title,
+                bodyText,
+                Ci.nsIPrompt.BUTTON_POS_0 * Ci.nsIPrompt.BUTTON_TITLE_IS_STRING
+              + Ci.nsIPrompt.BUTTON_POS_1 * Ci.nsIPrompt.BUTTON_TITLE_IS_STRING
+              + Ci.nsIPrompt.BUTTON_POS_1_DEFAULT,
+                acceptbutton,
+                refusebutton,
+                null,
+                null,
+                { value: false });
+
       Services.prefs.setBoolPref("app.crashreporter.prompted", true);
-      DialogUI.importModal(document, "chrome://browser/content/prompt/crash.xul");
-      return;
+
+      if (buttonPressed == 0) {
+        Services.prefs.setBoolPref('app.crashreporter.autosubmit', true);
+        BrowserUI.crashReportingPrefChanged(true);
+        shouldReport = true;
+      } else {
+        Services.prefs.setBoolPref('app.crashreporter.autosubmit', false);
+        BrowserUI.crashReportingPrefChanged(false);
+      }
     }
 
     // We've already prompted, return if the user doesn't want to report.
-    if (!shouldReport && didPrompt) {
+    if (!shouldReport) {
       return;
     }
 
     Util.dumpLn("Submitting last crash id:", lastCrashID);
     try {
       this.CrashSubmit.submit(lastCrashID);
     } catch (ex) {
       Util.dumpLn(ex);
@@ -729,21 +755,16 @@ var BrowserUI = {
     }
 
     // Check open popups
     if (DialogUI._popup) {
       DialogUI._hidePopup();
       return;
     }
 
-    // Check open modal elements
-    if (DialogUI.modals.length > 0) {
-      return;
-    }
-
     // Check open panel
     if (PanelUI.isVisible) {
       PanelUI.hide();
       return;
     }
 
     // Check content helper
     if (FindHelperUI.isActive) {
@@ -1104,17 +1125,16 @@ var StartUI = {
     Elements.startUI.addEventListener("click", this, false);
     Elements.startUI.addEventListener("MozMousePixelScroll", this, false);
 
     this.sections.forEach(function (sectionName) {
       let section = window[sectionName];
       if (section.init)
         section.init();
     });
-
   },
 
   uninit: function() {
     this.sections.forEach(function (sectionName) {
       let section = window[sectionName];
       if (section.uninit)
         section.uninit();
     });
@@ -1311,76 +1331,16 @@ var PanelUI = {
 var DialogUI = {
   _popup: null,
 
   init: function() {
     window.addEventListener("mousedown", this, true);
   },
 
   /*******************************************
-   * Modal popups
-   */
-
-  get modals() {
-    return document.getElementsByClassName("modal-block");
-  },
-
-  importModal: function importModal(aParent, aSrc, aArguments) {
-  // load the dialog with a synchronous XHR
-    let xhr = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"].createInstance();
-    xhr.open("GET", aSrc, false);
-    xhr.overrideMimeType("text/xml");
-    xhr.send(null);
-    if (!xhr.responseXML)
-      return null;
-
-    let currentNode;
-    let nodeIterator = xhr.responseXML.createNodeIterator(xhr.responseXML, NodeFilter.SHOW_TEXT, null, false);
-    while (!!(currentNode = nodeIterator.nextNode())) {
-      let trimmed = currentNode.nodeValue.replace(/^\s\s*/, "").replace(/\s\s*$/, "");
-      if (!trimmed.length)
-        currentNode.parentNode.removeChild(currentNode);
-    }
-
-    let doc = xhr.responseXML.documentElement;
-
-    let dialog  = null;
-
-    // we need to insert before context-container if we want allow pasting (using
-    //  the context menu) into dialogs
-    let contentMenuContainer = document.getElementById("context-container");
-    let parentNode = contentMenuContainer.parentNode;
-
-    // emit DOMWillOpenModalDialog event
-    let event = document.createEvent("Events");
-    event.initEvent("DOMWillOpenModalDialog", true, false);
-    let dispatcher = aParent || getBrowser();
-    dispatcher.dispatchEvent(event);
-
-    // create a full-screen semi-opaque box as a background or reuse
-    // the existing one.
-    let back = document.getElementById("dialog-modal-block");
-    if (!back) {
-      back = document.createElement("box");
-    } else {
-      while (back.hasChildNodes()) {
-        back.removeChild(back.firstChild);
-      }
-    }
-    back.setAttribute("class", "modal-block");
-    back.setAttribute("id", "dialog-modal-block");
-    dialog = back.appendChild(document.importNode(doc, true));
-    parentNode.insertBefore(back, contentMenuContainer);
-
-    dialog.arguments = aArguments;
-    dialog.parent = aParent;
-    return dialog;
-  },
-
-  /*******************************************
    * Popups
    */
 
   pushPopup: function pushPopup(aPanel, aElements, aParent) {
     this._hidePopup();
     this._popup =  { "panel": aPanel,
                      "elements": (aElements instanceof Array) ? aElements : [aElements] };
     this._dispatchPopupChanged(true, aPanel);
--- a/browser/metro/base/tests/mochitest/browser_crashprompt.js
+++ b/browser/metro/base/tests/mochitest/browser_crashprompt.js
@@ -25,33 +25,38 @@ var newFactory = {
     return new MockAppInfo().QueryInterface(aIID);
   },
   lockFactory: function(aLock) {
     throw Components.results.NS_ERROR_NOT_IMPLEMENTED;
   },
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIFactory])
 };
 
+let oldPrompt;
 function initMockAppInfo() {
   var registrar = Cm.QueryInterface(Ci.nsIComponentRegistrar);
   gAppInfoClassID = registrar.contractIDToCID(CONTRACT_ID);
   gIncOldFactory = Cm.getClassObject(Cc[CONTRACT_ID], Ci.nsIFactory);
   registrar.unregisterFactory(gAppInfoClassID, gIncOldFactory);
   var components = [MockAppInfo];
   registrar.registerFactory(gAppInfoClassID, "", CONTRACT_ID, newFactory);
   gIncOldFactory = Cm.getClassObject(Cc[CONTRACT_ID], Ci.nsIFactory);
+
+  oldPrompt = Services.prompt;
 }
 
 function cleanupMockAppInfo() {
   if (gIncOldFactory) {
     var registrar = Cm.QueryInterface(Ci.nsIComponentRegistrar);
     registrar.unregisterFactory(gAppInfoClassID, newFactory);
     registrar.registerFactory(gAppInfoClassID, "", CONTRACT_ID, gIncOldFactory);
   }
   gIncOldFactory = null;
+
+  Services.prompt = oldPrompt;
 }
 
 MockAppInfo.prototype = {
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIXULRuntime]),
   get lastRunCrashID() {
     gMockAppInfoQueried = true;
     return this.crashid;
   },
@@ -69,90 +74,82 @@ gTests.push({
     Services.prefs.setBoolPref('app.crashreporter.autosubmit', aValue);
   },
   get promptedPref() {
     return Services.prefs.getBoolPref("app.crashreporter.prompted");
   },
   set promptedPref(aValue) {
     Services.prefs.setBoolPref('app.crashreporter.prompted', aValue);
   },
-  get dialog() {
-    return document.getElementById("crash-prompt-dialog");
-  },
 
   run: function() {
     MockAppInfo.crashid = "testid";
 
-    // never prompted, autosubmit off. We should prompt.
+    // For the first set of tests, we want to be able to simulate that a
+    // specific button of the crash reporter prompt was pressed
+    Services.prompt = {
+      confirmEx: function() {
+        return this.retVal;
+      }
+    };
+
+    // Test 1:
+    // Pressing cancel button on the crash reporter prompt
+
+    // Set the crash reporter prefs to indicate that the prompt should appear
     this.autosubmitPref = false;
     this.promptedPref = false;
 
-    BrowserUI.startupCrashCheck();
-
-    yield waitForMs(100);
-
-    ok(this.dialog, "prompt dialog exists");
-    ok(!this.dialog.hidden, "prompt dialog is visible");
+    // We will simulate that "button 1" (which should be the cancel button)
+    // was pressed
+    Services.prompt.retVal = 1;
 
-    // user refuses crash reporting opt-in
-    let refuseButton = document.getElementById("crash-button-refuse");
-    sendElementTap(window, refuseButton, 20, 20);
-
-    yield waitForCondition(() => this.dialog == null);
-
+    BrowserUI.startupCrashCheck();
     ok(!this.autosubmitPref, "auto submit disabled?");
     ok(this.promptedPref, "prompted should be true");
 
-    // never prompted, autosubmit off. We should prompt.
+
+    // Test 2:
+    // Pressing 'ok' button on the crash reporter prompt
+
+    // Set the crash reporter prefs to indicate that the prompt should appear
     this.autosubmitPref = false;
     this.promptedPref = false;
 
     // should query on the first call to startupCrashCheck
     gMockAppInfoQueried = false;
 
-    BrowserUI.startupCrashCheck();
-
-    yield waitForMs(100);
-
-    ok(this.dialog, "prompt dialog exists");
-    ok(!this.dialog.hidden, "prompt dialog is visible");
-    ok(gMockAppInfoQueried, "id queried");
+    // We will simulate that "button 0" (which should be the OK button)
+    // was pressed
+    Services.prompt.retVal = 0;
 
-    // should query again when the user submits
-    gMockAppInfoQueried = false;
-
-    // user accepts crash reporting opt-in
-    let submitButton = document.getElementById("crash-button-accept");
-    sendElementTap(window, submitButton, 20, 20);
-
-    yield waitForCondition(() => this.dialog == null);
-
+    BrowserUI.startupCrashCheck();
+    ok(gMockAppInfoQueried, "id queried");
     ok(this.autosubmitPref, "auto submit enabled?");
     ok(this.promptedPref, "prompted should be true");
-    ok(gMockAppInfoQueried, "id queried");
+
 
-    // prompted, auto-submit off. We shouldn't prompt.
+    // For the remaining tests, attempting to launch the crash reporter
+    // prompt would be incorrect behavior
+    Services.prompt.confirmEx = function() {
+      ok(false, "Should not attempt to launch crash reporter prompt");
+    };
+
+    // Test 3:
+    // Prompt should not appear if pref indicates that user has already
+    // been prompted
     this.autosubmitPref = false;
     this.promptedPref = true;
-
     BrowserUI.startupCrashCheck();
 
-    yield waitForMs(100);
-
-    ok(!this.dialog, "prompt dialog does not exists");
-
-    // never prompted, autosubmit *on*. We shouldn't prompt.
+    // Test 4:
+    // Prompt should not appear if pref indicates that autosubmit is on
     this.autosubmitPref = true;
     this.promptedPref = false;
-
     BrowserUI.startupCrashCheck();
-
-    yield waitForMs(100);
-
-    ok(!this.dialog, "prompt dialog does not exists");
   }
 });
 
 function test() {
   if (!CrashReporter.enabled) {
     info("crash reporter prompt tests didn't run, CrashReporter.enabled is false.");
     return;
   }
--- a/browser/metro/locales/jar.mn
+++ b/browser/metro/locales/jar.mn
@@ -15,17 +15,16 @@
   locale/browser/config.dtd               (%chrome/config.dtd)
   locale/browser/preferences.dtd          (%chrome/preferences.dtd)
   locale/browser/aboutPanel.dtd           (%chrome/aboutPanel.dtd)
   locale/browser/checkbox.dtd             (%chrome/checkbox.dtd)
   locale/browser/sync.dtd                 (%chrome/sync.dtd)
   locale/browser/sync.properties          (%chrome/sync.properties)
   locale/browser/passwordmgr.properties   (%chrome/passwordmgr.properties)
   locale/browser/phishing.dtd             (%chrome/phishing.dtd)
-  locale/browser/crashprompt.dtd          (%chrome/crashprompt.dtd)
   locale/browser/crashprompt.properties   (%chrome/crashprompt.properties)
   locale/browser/aboutAddons.dtd          (%chrome/aboutAddons.dtd)
 
 @AB_CD@.jar:
 % locale browser @AB_CD@ %locale/browser/
   locale/browser/bookmarks.json           (bookmarks.json)
 
 #