Bug 817294 - Intermittent browser_styleeditor_loading.js | content document is still loading - Didn't expect complete, but got it | style editor root element has 'loading' class name | new style sheet button is disabled | import button is disabled

browser/devtools/styleeditor/StyleEditor.jsm

@@ -130,7 +130,7 @@ StyleEditor.prototype = {
   {
     let document = this.contentDocument;
     if (this._styleSheetIndex == -1) {
-      for (let i = 0; i < document.styleSheets.length; ++i) {
+      for (let i = 0; i < document.styleSheets.length; i++) {
         if (document.styleSheets[i] == this.styleSheet) {
           this._styleSheetIndex = i;
           break;
@@ -1004,8 +1004,9 @@ StyleEditor.prototype = {
 
     // copy the list of listeners to allow adding/removing listeners in handlers
     let listeners = this._actionListeners.concat();
+
     // trigger all listeners that have this action handler
-    for (let i = 0; i < listeners.length; ++i) {
+    for (let i = 0; i < listeners.length; i++) {
       let listener = listeners[i];
       let actionHandler = listener["on" + aName];
       if (actionHandler) {

browser/devtools/styleeditor/StyleEditorChrome.jsm

@@ -16,6 +16,7 @@ Cu.import("resource://gre/modules/PluralForm.jsm");
 Cu.import("resource:///modules/devtools/StyleEditor.jsm");
 Cu.import("resource:///modules/devtools/StyleEditorUtil.jsm");
 Cu.import("resource:///modules/devtools/SplitView.jsm");
+Cu.import("resource://gre/modules/commonjs/promise/core.js");
 
 const STYLE_EDITOR_TEMPLATE = "stylesheet";
 
@@ -43,22 +44,32 @@ this.StyleEditorChrome = function StyleEditorChrome(aRoot, aContentWindow)
   this._editors = [];
   this._listeners = []; // @see addChromeListener
 
-  this._contentWindow = null;
+  // Store the content window so that we can call the real contentWindow setter
-  this._isContentAttached = false;
+  // in the open method.
+  this._contentWindowTemp = aContentWindow;
 
+  this._contentWindow = null;
+}
+
+StyleEditorChrome.prototype = {
+  _styleSheetToSelect: null,
+
+  open: function() {
+    let deferred = Promise.defer();
     let initializeUI = function (aEvent) {
       if (aEvent) {
         this._window.removeEventListener("load", initializeUI, false);
       }
-
       let viewRoot = this._root.parentNode.querySelector(".splitview-root");
       this._view = new SplitView(viewRoot);
-
       this._setupChrome();
 
-    // attach to the content window
+      // We need to juggle arount the contentWindow items because we need to
-    this.contentWindow = aContentWindow;
+      // trigger the setter at the appropriate time.
-    this._contentWindowID = null;
+      this.contentWindow = this._contentWindowTemp; // calls setter
+      this._contentWindowTemp = null;
+
+      deferred.resolve();
     }.bind(this);
 
     if (this._document.readyState == "complete") {
@@ -66,10 +77,9 @@ this.StyleEditorChrome = function StyleEditorChrome(aRoot, aContentWindow)
     } else {
       this._window.addEventListener("load", initializeUI, false);
     }
-}
 
-StyleEditorChrome.prototype = {
+    return deferred.promise;
-  _styleSheetToSelect: null,
+  },
 
   /**
    * Retrieve the content window attached to this chrome.
@@ -150,15 +160,6 @@ StyleEditorChrome.prototype = {
     return this._contentWindow ? this._contentWindow.document : null;
   },
 
-  /**
-    * Retrieve whether the content has been attached and StyleEditor instances
-    * exist for all of its stylesheets.
-    *
-    * @return boolean
-    * @see addChromeListener
-    */
-  get isContentAttached() this._isContentAttached,
-
   /**
    * Retrieve an array with the StyleEditor instance for each live style sheet,
    * ordered by style sheet index.
@@ -180,14 +181,6 @@ StyleEditorChrome.prototype = {
    * Add a listener for StyleEditorChrome events.
    *
    * The listener implements IStyleEditorChromeListener := {
-   *   onContentAttach:        Called when a content window has been attached.
-   *                           All editors are instantiated, though they might
-   *                           not be loaded yet.
-   *                           Arguments: (StyleEditorChrome aChrome)
-   *                           @see contentWindow
-   *                           @see StyleEditor.isLoaded
-   *                           @see StyleEditor.addActionListener
-   *
    *   onContentDetach:        Called when the content window has been detached.
    *                           Arguments: (StyleEditorChrome aChrome)
    *                           @see contentWindow
@@ -287,7 +280,7 @@ StyleEditorChrome.prototype = {
 
     // (re)enable UI
     let matches = this._root.querySelectorAll("toolbarbutton,input,select");
-    for (let i = 0; i < matches.length; ++i) {
+    for (let i = 0; i < matches.length; i++) {
       matches[i].removeAttribute("disabled");
     }
   },
@@ -305,7 +298,7 @@ StyleEditorChrome.prototype = {
     this._document.title = _("chromeWindowTitle",
       document.title || document.location.href);
 
-    for (let i = 0; i < document.styleSheets.length; ++i) {
+    for (let i = 0; i < document.styleSheets.length; i++) {
       let styleSheet = document.styleSheets[i];
 
       let editor = new StyleEditor(document, styleSheet);
@@ -313,8 +306,6 @@ StyleEditorChrome.prototype = {
       this._editors.push(editor);
     }
 
-    this._triggerChromeListeners("ContentAttach");
-
     // Queue editors loading so that ContentAttach is consistently triggered
     // right after all editor instances are available (this.editors) but are
     // NOT loaded/ready yet. This also helps responsivity during loading when
@@ -353,42 +344,35 @@ StyleEditorChrome.prototype = {
 
     let select = function DEC_select(aEditor) {
       let sheet = this._styleSheetToSelect.sheet;
-      let line = this._styleSheetToSelect.line;
+      let line = this._styleSheetToSelect.line || 1;
-      let col = this._styleSheetToSelect.col;
+      let col = this._styleSheetToSelect.col || 1;
-      let summary = sheet ? this.getSummaryElementForEditor(aEditor)
-                          : this._view.getSummaryElementByOrdinal(0);
-
-      if (line) {
-        col = col || 1;
 
       if (!aEditor.sourceEditor) {
-          // If a line or column was specified we move the caret appropriately.
+        let onAttach = function SEC_selectSheet_onAttach() {
-          let self = this;
-          aEditor.addActionListener({
-            onAttach: function SEC_selectSheet_onAttach()
-            {
           aEditor.removeActionListener(this);
-              self.selectedStyleSheetIndex = aEditor.styleSheetIndex;
+          this.selectedStyleSheetIndex = aEditor.styleSheetIndex;
           aEditor.sourceEditor.setCaretPosition(line - 1, col - 1);
 
-              let newSheet = self._styleSheetToSelect.sheet;
+          let newSheet = this._styleSheetToSelect.sheet;
-              let newLine = self._styleSheetToSelect.line;
+          let newLine = this._styleSheetToSelect.line;
-              let newCol = self._styleSheetToSelect.col;
+          let newCol = this._styleSheetToSelect.col;
-              self._styleSheetToSelect = null;
+          this._styleSheetToSelect = null;
           if (newSheet != sheet) {
-                self._window.setTimeout(self.selectStyleSheet.bind(self, newSheet, newLine, newCol), 0);
+              this.selectStyleSheet.bind(this, newSheet, newLine, newCol);
-              }
           }
+        }.bind(this);
+
+        aEditor.addActionListener({
+          onAttach: onAttach
         });
       } else {
         // If a line or column was specified we move the caret appropriately.
         aEditor.sourceEditor.setCaretPosition(line - 1, col - 1);
         this._styleSheetToSelect = null;
       }
-      } else {
-        this._styleSheetToSelect = null;
-      }
 
+        let summary = sheet ? this.getSummaryElementForEditor(aEditor)
+                            : this._view.getSummaryElementByOrdinal(0);
         this._view.activeSummary = summary;
       this.selectedStyleSheetIndex = aEditor.styleSheetIndex;
     }.bind(this);
@@ -404,6 +388,7 @@ StyleEditorChrome.prototype = {
           if ((sheet && aEditor.styleSheet == sheet) ||
               (aEditor.styleSheetIndex == 0 && sheet == null)) {
             aChrome.removeChromeListener(this);
+            aEditor.addActionListener(self);
             select(aEditor);
           }
         }
@@ -430,7 +415,7 @@ StyleEditorChrome.prototype = {
   _disableChrome: function SEC__disableChrome()
   {
     let matches = this._root.querySelectorAll("button,toolbarbutton,textbox");
-    for (let i = 0; i < matches.length; ++i) {
+    for (let i = 0; i < matches.length; i++) {
       matches[i].setAttribute("disabled", "disabled");
     }
 

browser/devtools/styleeditor/StyleEditorPanel.jsm

@@ -39,10 +39,14 @@ StyleEditorPanel.prototype = {
    */
   open: function StyleEditor_open() {
     let contentWin = this._toolbox.target.window;
-    this.setPage(contentWin);
+    let deferred = Promise.defer();
-    this.isReady = true;
 
-    return Promise.resolve(this);
+    this.setPage(contentWin).then(function() {
+      this.isReady = true;
+      deferred.resolve(this);
+    }.bind(this));
+
+    return deferred.promise;
   },
 
   /**
@@ -66,12 +70,16 @@ StyleEditorPanel.prototype = {
   setPage: function StyleEditor_setPage(contentWindow) {
     if (this._panelWin.styleEditorChrome) {
       this._panelWin.styleEditorChrome.contentWindow = contentWindow;
+      this.selectStyleSheet(null, null, null);
     } else {
       let chromeRoot = this._panelDoc.getElementById("style-editor-chrome");
       let chrome = new StyleEditorChrome(chromeRoot, contentWindow);
+      let promise = chrome.open();
+
       this._panelWin.styleEditorChrome = chrome;
-    }
       this.selectStyleSheet(null, null, null);
+      return promise;
+    }
   },
 
   /**

browser/devtools/styleeditor/StyleEditorUtil.jsm

@@ -152,7 +152,7 @@ this.wire = function wire(aRoot, aSelectorOrElement, aDescriptor)
     aDescriptor = {events: {click: aDescriptor}};
   }
 
-  for (let i = 0; i < matches.length; ++i) {
+  for (let i = 0; i < matches.length; i++) {
     let element = matches[i];
     forEach(aDescriptor.events, function (aName, aHandler) {
       element.addEventListener(aName, aHandler, false);

browser/devtools/styleeditor/test/Makefile.in

@@ -18,7 +18,7 @@ _BROWSER_TEST_FILES = \
                  browser_styleeditor_cmd_edit.html \
                  browser_styleeditor_import.js \
                  browser_styleeditor_init.js \
-                 $(filter disabled-temporarily--bug-817294, browser_styleeditor_loading.js) \
+                 browser_styleeditor_loading.js \
                  browser_styleeditor_new.js \
                  browser_styleeditor_passedinsheet.js \
                  browser_styleeditor_pretty.js \
@@ -32,6 +32,7 @@ _BROWSER_TEST_FILES = \
                  four.html \
                  head.js \
                  helpers.js \
+                 longload.html \
                  media.html \
                  media-small.css \
                  minified.html \

browser/devtools/styleeditor/test/browser_styleeditor_import.js

@@ -19,12 +19,9 @@ function test()
 
   addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) {
     aChrome.addChromeListener({
-      onContentAttach: run,
       onEditorAdded: testEditorAdded
     });
-    if (aChrome.isContentAttached) {
     run(aChrome);
-    }
   });
 
   content.location = TESTCASE_URI;

browser/devtools/styleeditor/test/browser_styleeditor_init.js

@@ -9,23 +9,17 @@ function test()
 {
   waitForExplicitFinish();
 
-  addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) {
+  launchStyleEditorChrome(function(aChrome) {
     aChrome.addChromeListener({
-      onContentAttach: run,
       onEditorAdded: testEditorAdded
     });
-    if (aChrome.isContentAttached) {
     run(aChrome);
-    }
   });
-
   content.location = TESTCASE_URI;
 }
 
-let gContentAttachHandled = false;
 function run(aChrome)
 {
-  gContentAttachHandled = true;
   is(aChrome.contentWindow.document.readyState, "complete",
      "content document is complete");
 
@@ -42,11 +36,6 @@ function run(aChrome)
 let gEditorAddedCount = 0;
 function testEditorAdded(aChrome, aEditor)
 {
-  if (!gEditorAddedCount) {
-    is(gContentAttachHandled, true,
-       "ContentAttach event triggered before EditorAdded");
-  }
-
   if (aEditor.styleSheetIndex == 0) {
     gEditorAddedCount++;
     testFirstStyleSheetEditor(aChrome, aEditor);

browser/devtools/styleeditor/test/browser_styleeditor_loading.js

@@ -2,7 +2,7 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
-const TESTCASE_URI = TEST_BASE + "simple.html";
+const TESTCASE_URI = TEST_BASE + "longload.html";
 
 
 function test()
@@ -13,38 +13,10 @@ function test()
 
   // launch Style Editor right when the tab is created (before load)
   // this checks that the Style Editor still launches correctly when it is opened
-  // *while* the page is still loading
+  // *while* the page is still loading. The Style Editor should not signal that
+  // it is loaded until the accompanying content page is loaded.
   launchStyleEditorChrome(function (aChrome) {
     content.location = TESTCASE_URI;
-    executeSoon(function() {
-      isnot(gBrowser.selectedBrowser.contentWindow.document.readyState, "complete",
-            "content document is still loading");
-
-      let root = gChromeWindow.document.querySelector(".splitview-root");
-      ok(root.classList.contains("loading"),
-        "style editor root element has 'loading' class name");
-
-      let button = gChromeWindow.document.querySelector(".style-editor-newButton");
-      ok(button.hasAttribute("disabled"),
-        "new style sheet button is disabled");
-
-      button = gChromeWindow.document.querySelector(".style-editor-importButton");
-      ok(button.hasAttribute("disabled"),
-        "import button is disabled");
-
-      if (!aChrome.isContentAttached) {
-        aChrome.addChromeListener({
-          onContentAttach: run
-        });
-      } else {
-        run(aChrome);
-      }
-    });
-  });
-}
-
-function run(aChrome)
-{
       is(aChrome.contentWindow.document.readyState, "complete",
          "content document is complete");
 
@@ -61,4 +33,5 @@ function run(aChrome)
          "import button is enabled");
 
       finish();
+  });
 }

browser/devtools/styleeditor/test/browser_styleeditor_new.js

@@ -13,12 +13,9 @@ function test()
 
   addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) {
     aChrome.addChromeListener({
-      onContentAttach: run,
       onEditorAdded: testEditorAdded
     });
-    if (aChrome.isContentAttached) {
     run(aChrome);
-    }
   });
 
   content.location = TESTCASE_URI;

browser/devtools/styleeditor/test/browser_styleeditor_private_perwindowpb.js

@@ -19,13 +19,7 @@ function test() {
       aWindow.gBrowser.selectedBrowser.removeEventListener("load", onLoad, true);
       cache.evictEntries(Ci.nsICache.STORE_ANYWHERE);
       launchStyleEditorChromeFromWindow(aWindow, function(aChrome) {
-        if (aChrome.isContentAttached) {
         onEditorAdded(aChrome, aChrome.editors[0]);
-        } else {
-          aChrome.addChromeListener({
-            onEditorAdded: onEditorAdded
-          });
-        }
       });
     }, true);
 

browser/devtools/styleeditor/test/browser_styleeditor_sv_keynav.js

@@ -10,12 +10,7 @@ function test()
   waitForExplicitFinish();
 
   addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) {
-    aChrome.addChromeListener({
-      onContentAttach: run
-    });
-    if (aChrome.isContentAttached) {
     run(aChrome);
-    }
   });
 
   content.location = TESTCASE_URI;

browser/devtools/styleeditor/test/browser_styleeditor_sv_resize.js

@@ -12,15 +12,8 @@ function test()
   waitForExplicitFinish();
 
   addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) {
-    if (aChrome.isContentAttached) {
     run(aChrome);
-    } else {
-      aChrome.addChromeListener({
-        onContentAttach: run
   });
-    }
-  });
-
   content.location = TESTCASE_URI;
 }
 
@@ -31,13 +24,13 @@ function run(aChrome)
 
   aChrome.editors[0].addActionListener({
     onAttach: function onEditorAttached(aEditor) {
+      executeSoon(function () {
+        waitForFocus(function () {
+          // queue a resize to inverse aspect ratio
+          // this will trigger a detach and reattach (to workaround bug 254144)
           let originalSourceEditor = aEditor.sourceEditor;
           aEditor.sourceEditor.setCaretOffset(4); // to check the caret is preserved
 
-      // queue a resize to inverse aspect ratio
-      // this will trigger a detach and reattach (to workaround bug 254144)
-      executeSoon(function () {
-        waitForFocus(function () {
           gOriginalWidth = gChromeWindow.outerWidth;
           gOriginalHeight = gChromeWindow.outerHeight;
           gChromeWindow.resizeTo(120, 480);

browser/devtools/styleeditor/test/longload.html

@@ -0,0 +1,28 @@
+<!doctype html>
+<html>
+<head>
+  <title>Long load</title>
+  <link rel="stylesheet" charset="UTF-8" type="text/css" media="screen" href="simple.css"/>
+  <style type="text/css">
+  body {
+    background: white;
+  }
+
+  div {
+    font-size: 4em;
+  }
+
+  div > span {
+     text-decoration: underline;
+  }
+  </style>
+</head>
+<body>
+  Time passes:
+  <script>
+    for (i = 0; i < 30000; i++) {
+      document.write("<br>...");
+    }
+  </script>
+</body>
+</html>