Bug 1040882 - Only delete transition rule after updating stylesheet in style editor. r=jwalker
authorHeather Arthur <fayearthur@gmail.com>
Thu, 31 Jul 2014 11:25:00 -0400
changeset 197499 b463979c8dd3e538a7e98d7b52b8b3c3a8fb1498
parent 197498 7b02f8b19ee44ffa5dabb9bd5d7d6bc55497dc72
child 197500 09b0fd7d826113a07962510de69dc3a3f0e8e1ac
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersjwalker
bugs1040882
milestone34.0a1
Bug 1040882 - Only delete transition rule after updating stylesheet in style editor. r=jwalker
browser/devtools/styleeditor/test/browser.ini
browser/devtools/styleeditor/test/browser_styleeditor_transition_rule.js
toolkit/devtools/server/actors/stylesheets.js
--- a/browser/devtools/styleeditor/test/browser.ini
+++ b/browser/devtools/styleeditor/test/browser.ini
@@ -62,8 +62,9 @@ skip-if = os == "linux" || "mac" # bug 9
 [browser_styleeditor_pretty.js]
 [browser_styleeditor_private_perwindowpb.js]
 [browser_styleeditor_reload.js]
 [browser_styleeditor_sv_keynav.js]
 [browser_styleeditor_sv_resize.js]
 [browser_styleeditor_selectstylesheet.js]
 [browser_styleeditor_sourcemaps.js]
 [browser_styleeditor_sourcemap_watching.js]
+[browser_styleeditor_transition_rule.js]
new file mode 100644
--- /dev/null
+++ b/browser/devtools/styleeditor/test/browser_styleeditor_transition_rule.js
@@ -0,0 +1,48 @@
+/* vim: set ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+const TESTCASE_URI = TEST_BASE_HTTPS + "simple.html";
+
+waitForExplicitFinish();
+
+const NEW_RULE = "body { background-color: purple; }";
+
+let test = asyncTest(function*() {
+  let {UI} = yield addTabAndOpenStyleEditors(2, null, TESTCASE_URI);
+
+  is(UI.editors.length, 2, "correct number of editors");
+
+  let editor = UI.editors[0];
+  yield openEditor(editor);
+
+  // Set text twice in a row
+  let styleChanges = listenForStyleChange(editor.styleSheet);
+
+  editor.sourceEditor.setText(NEW_RULE);
+  editor.sourceEditor.setText(NEW_RULE + " ");
+
+  yield styleChanges;
+
+  let sheet = content.document.styleSheets[0];
+
+  // Test that we removed the transition rule, but kept the rule we added
+  is(sheet.cssRules.length, 1, "only one rule in stylesheet");
+  is(sheet.cssRules[0].cssText, NEW_RULE,
+     "stylesheet only contains rule we added");
+});
+
+/* Helpers */
+
+function openEditor(editor) {
+  let link = editor.summary.querySelector(".stylesheet-name");
+  link.click();
+
+  return editor.getSourceEditor();
+}
+
+function listenForStyleChange(sheet) {
+  let deferred = promise.defer();
+  sheet.on("style-applied", deferred.resolve);
+  return deferred.promise;
+}
--- a/toolkit/devtools/server/actors/stylesheets.js
+++ b/toolkit/devtools/server/actors/stylesheets.js
@@ -19,18 +19,19 @@ const protocol = require("devtools/serve
 const {Arg, Option, method, RetVal, types} = protocol;
 const {LongStringActor, ShortLongString} = require("devtools/server/actors/string");
 
 loader.lazyGetter(this, "CssLogic", () => require("devtools/styleinspector/css-logic").CssLogic);
 
 let TRANSITION_CLASS = "moz-styleeditor-transitioning";
 let TRANSITION_DURATION_MS = 500;
 let TRANSITION_BUFFER_MS = 1000;
-let TRANSITION_RULE = "\
-:root.moz-styleeditor-transitioning, :root.moz-styleeditor-transitioning * {\
+let TRANSITION_RULE_SELECTOR =
+".moz-styleeditor-transitioning:root, .moz-styleeditor-transitioning:root *";
+let TRANSITION_RULE = TRANSITION_RULE_SELECTOR + " {\
 transition-duration: " + TRANSITION_DURATION_MS + "ms !important; \
 transition-delay: 0ms !important;\
 transition-timing-function: ease-out !important;\
 transition-property: all !important;\
 }";
 
 let LOAD_ERROR = "error-load";
 
@@ -902,41 +903,40 @@ let StyleSheetActor = protocol.ActorClas
     }
   }),
 
   /**
    * Insert a catch-all transition rule into the document. Set a timeout
    * to remove the rule after a certain time.
    */
   _insertTransistionRule: function() {
-    // Insert the global transition rule
-    // Use a ref count to make sure we do not add it multiple times.. and remove
-    // it only when all pending StyleSheets-generated transitions ended.
-    if (this._transitionRefCount == 0) {
-      this.rawSheet.insertRule(TRANSITION_RULE, this.rawSheet.cssRules.length);
-      this.document.documentElement.classList.add(TRANSITION_CLASS);
-    }
+    this.document.documentElement.classList.add(TRANSITION_CLASS);
 
-    this._transitionRefCount++;
+    // We always add the rule since we've just reset all the rules
+    this.rawSheet.insertRule(TRANSITION_RULE, this.rawSheet.cssRules.length);
 
     // Set up clean up and commit after transition duration (+buffer)
     // @see _onTransitionEnd
-    this.window.setTimeout(this._onTransitionEnd.bind(this),
-                           TRANSITION_DURATION_MS + TRANSITION_BUFFER_MS);
+    this.window.clearTimeout(this._transitionTimeout);
+    this._transitionTimeout = this.window.setTimeout(this._onTransitionEnd.bind(this),
+                              TRANSITION_DURATION_MS + TRANSITION_BUFFER_MS);
   },
 
   /**
    * This cleans up class and rule added for transition effect and then
    * notifies that the style has been applied.
    */
   _onTransitionEnd: function()
   {
-    if (--this._transitionRefCount == 0) {
-      this.document.documentElement.classList.remove(TRANSITION_CLASS);
-      this.rawSheet.deleteRule(this.rawSheet.cssRules.length - 1);
+    this.document.documentElement.classList.remove(TRANSITION_CLASS);
+
+    let index = this.rawSheet.cssRules.length - 1;
+    let rule = this.rawSheet.cssRules[index];
+    if (rule.selectorText == TRANSITION_RULE_SELECTOR) {
+      this.rawSheet.deleteRule(index);
     }
 
     events.emit(this, "style-applied");
   }
 })
 
 /**
  * Find the line/column for a rule.