Bug 885246 - Delay enabling the accept button on the bookmarks properties dialog until after init is complete. r?adw draft
authorMark Banner <standard8@mozilla.com>
Tue, 08 Aug 2017 14:01:57 +0100
changeset 649825 d58a6edc813f2315305fd63ad4aa9835ea3e272f
parent 649622 7dddbd85047c6dc73ddbe1e423cd643a217845b3
child 649826 1bd5591639e846486922732954fd0618a6f5e16e
push id75169
push userbmo:standard8@mozilla.com
push dateMon, 21 Aug 2017 11:04:23 +0000
reviewersadw
bugs885246
milestone57.0a1
Bug 885246 - Delay enabling the accept button on the bookmarks properties dialog until after init is complete. r?adw The initialisation can sometimes take a long time. To avoid data gathering being incomplete, we disable the accept button until it has completed. MozReview-Commit-ID: 3gnROdZkYae
browser/components/places/content/bookmarkProperties.js
--- a/browser/components/places/content/bookmarkProperties.js
+++ b/browser/components/places/content/bookmarkProperties.js
@@ -257,22 +257,40 @@ var BookmarkPropertiesPanel = {
     // get the title from History
     return PlacesUtils.history.getPageTitle(aURI);
   },
 
   /**
    * This method should be called by the onload of the Bookmark Properties
    * dialog to initialize the state of the panel.
    */
-  async onDialogLoad() {
+  onDialogLoad() {
     this._determineItemInfo();
 
     document.title = this._getDialogTitle();
-    var acceptButton = document.documentElement.getButton("accept");
+
+    // Disable the buttons until we have all the information required.
+    let acceptButton = document.documentElement.getButton("accept");
+    acceptButton.disabled = true;
+
+    // Allow initialization to complete in a truely async manner so that we're
+    // not blocking the main thread.
+    this._initDialog().catch(ex => {
+      Components.utils.reportError(`Failed to initialize dialog: ${ex}`);
+    });
+  },
+
+  /**
+   * Initializes the dialog, gathering the required bookmark data. This function
+   * will enable the accept button (if appropraite) when it is complete.
+   */
+  async _initDialog() {
+    let acceptButton = document.documentElement.getButton("accept");
     acceptButton.label = this._getAcceptLabel();
+    let acceptButtonDisabled = false;
 
     // Do not use sizeToContent, otherwise, due to bug 90276, the dialog will
     // grow at every opening.
     // Since elements can be uncollapsed asynchronously, we must observe their
     // mutations and resize the dialog using a cached element size.
     this._height = window.outerHeight;
     this._mutationObserver = new MutationObserver(mutations => {
       for (let mutation of mutations) {
@@ -308,17 +326,17 @@ var BookmarkPropertiesPanel = {
 
     this._beginBatch();
 
     switch (this._action) {
       case ACTION_EDIT:
         gEditItemOverlay.initPanel({ node: this._node,
                                      hiddenRows: this._hiddenRows,
                                      focusedElement: "first" });
-        acceptButton.disabled = gEditItemOverlay.readOnly;
+        acceptButtonDisabled = gEditItemOverlay.readOnly;
         break;
       case ACTION_ADD:
         this._node = await this._promiseNewItem();
         // Edit the new item
         gEditItemOverlay.initPanel({ node: this._node,
                                      hiddenRows: this._hiddenRows,
                                      postData: this._postData,
                                      focusedElement: "first" });
@@ -328,31 +346,33 @@ var BookmarkPropertiesPanel = {
         // disabled by the input listener until the user fills the field.
         let locationField = this._element("locationField");
         if (locationField.value == "about:blank")
           locationField.value = "";
 
         // if this is an uri related dialog disable accept button until
         // the user fills an uri value.
         if (this._itemType == BOOKMARK_ITEM)
-          acceptButton.disabled = !this._inputIsValid();
+          acceptButtonDisabled = !this._inputIsValid();
         break;
     }
 
     if (!gEditItemOverlay.readOnly) {
       // Listen on uri fields to enable accept button if input is valid
       if (this._itemType == BOOKMARK_ITEM) {
         this._element("locationField")
             .addEventListener("input", this);
         if (this._isAddKeywordDialog) {
           this._element("keywordField")
               .addEventListener("input", this);
         }
       }
     }
+    // Only enable the accept button once we've finished everything.
+    acceptButton.disabled = acceptButtonDisabled;
   },
 
   // nsIDOMEventListener
   handleEvent: function BPP_handleEvent(aEvent) {
     var target = aEvent.target;
     switch (aEvent.type) {
       case "input":
         if (target.id == "editBMPanel_locationField" ||