Bug 1187443 - [Rule View] Remove preview value on start editing of a property r=bgrins
authorGabriel Luong <gabriel.luong@gmail.com>
Tue, 28 Jul 2015 14:26:18 -0700
changeset 286851 aa144e36fab0b56b256b1b54fc053288501091b3
parent 286850 b32f21db299c4916b1dbb5614401eecc6e951ab2
child 286852 5f27cc13a2c2e62fb18ee8dc1d5c9bade4862446
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1187443
milestone42.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 1187443 - [Rule View] Remove preview value on start editing of a property r=bgrins
browser/devtools/styleinspector/rule-view.js
browser/devtools/styleinspector/test/browser_ruleview_edit-property_04.js
browser/devtools/styleinspector/test/browser_ruleview_multiple-properties-unfinished_01.js
browser/devtools/styleinspector/test/head.js
--- a/browser/devtools/styleinspector/rule-view.js
+++ b/browser/devtools/styleinspector/rule-view.js
@@ -3158,17 +3158,18 @@ TextPropertyEditor.prototype = {
     // Populate the computed styles.
     this._updateComputed();
 
     // Update the rule property highlight.
     this.ruleView._updatePropertyHighlight(this);
   },
 
   _onStartEditing: function() {
-    this._previewValue(this.prop.value);
+    this.element.classList.remove("ruleview-overridden");
+    this.enable.style.visibility = "hidden";
   },
 
   /**
    * Populate the list of computed styles.
    */
   _updateComputed: function() {
     // Clear out existing viewers.
     while (this.computed.hasChildNodes()) {
@@ -3463,19 +3464,16 @@ TextPropertyEditor.prototype = {
    */
   _previewValue: function(aValue) {
     // Since function call is throttled, we need to make sure we are still
     // editing, and any selector modifications have been completed
     if (!this.editing || this.ruleEditor.isEditing) {
       return;
     }
 
-    this.element.classList.remove("ruleview-overridden");
-    this.enable.style.visibility = "hidden";
-
     let val = parseSingleValue(aValue);
     this.ruleEditor.rule.previewPropertyValue(this.prop, val.value,
                                               val.priority);
   },
 
   /**
    * Validate this property. Does it make sense for this value to be assigned
    * to this property name? This does not apply the property value
--- a/browser/devtools/styleinspector/test/browser_ruleview_edit-property_04.js
+++ b/browser/devtools/styleinspector/test/browser_ruleview_edit-property_04.js
@@ -1,16 +1,15 @@
 /* vim: set ft=javascript ts=2 et sw=2 tw=80: */
 /* Any copyright is dedicated to the Public Domain.
  http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
-// Tests that a disabled property is previewed when the property name or value
-// editor is focused and the property remains disabled when the escaping out of
+// Tests that a disabled property remains disabled when the escaping out of
 // the property editor.
 
 let TEST_URI = [
   "<style type='text/css'>",
   "#testid {",
   "  background-color: blue;",
   "}",
   "</style>",
@@ -34,42 +33,41 @@ function* testDisableProperty(inspector,
 
   let newValue = yield executeInContent("Test:GetRulePropertyValue", {
     styleSheetIndex: 0,
     ruleIndex: 0,
     name: "background-color"
   });
   is(newValue, "", "background-color should have been unset.");
 
-  yield testPreviewDisableProperty(view, ruleEditor, propEditor,
+  yield testEditDisableProperty(view, ruleEditor, propEditor,
     propEditor.nameSpan, "VK_ESCAPE");
-  yield testPreviewDisableProperty(view, ruleEditor, propEditor,
+  yield testEditDisableProperty(view, ruleEditor, propEditor,
     propEditor.valueSpan, "VK_ESCAPE");
-  yield testPreviewDisableProperty(view, ruleEditor, propEditor,
+  yield testEditDisableProperty(view, ruleEditor, propEditor,
     propEditor.valueSpan, "VK_TAB");
-  yield testPreviewDisableProperty(view, ruleEditor, propEditor,
+  yield testEditDisableProperty(view, ruleEditor, propEditor,
     propEditor.valueSpan, "VK_RETURN");
 }
 
-function* testPreviewDisableProperty(view, ruleEditor, propEditor,
+function* testEditDisableProperty(view, ruleEditor, propEditor,
     editableField, commitKey) {
   let editor = yield focusEditableField(view, editableField);
-  yield ruleEditor.rule._applyingModifications;
 
   ok(!propEditor.element.classList.contains("ruleview-overridden"),
     "property is not overridden.");
   is(propEditor.enable.style.visibility, "hidden",
     "property enable checkbox is hidden.");
 
   let newValue = yield executeInContent("Test:GetRulePropertyValue", {
     styleSheetIndex: 0,
     ruleIndex: 0,
     name: "background-color"
   });
-  is(newValue, "blue", "background-color should have been previewed.");
+  is(newValue, "", "background-color should remain unset.");
 
   let onBlur = once(editor.input, "blur");
   EventUtils.synthesizeKey(commitKey, {}, view.styleWindow);
   yield onBlur;
   yield ruleEditor.rule._applyingModifications;
 
   ok(!propEditor.prop.enabled, "property is disabled.");
   ok(propEditor.element.classList.contains("ruleview-overridden"),
@@ -79,10 +77,10 @@ function* testPreviewDisableProperty(vie
   ok(!propEditor.enable.getAttribute("checked"),
     "property enable checkbox is not checked.");
 
   newValue = yield executeInContent("Test:GetRulePropertyValue", {
     styleSheetIndex: 0,
     ruleIndex: 0,
     name: "background-color"
   });
-  is(newValue, "", "background-color should have been unset.");
+  is(newValue, "", "background-color should remain unset.");
 }
--- a/browser/devtools/styleinspector/test/browser_ruleview_multiple-properties-unfinished_01.js
+++ b/browser/devtools/styleinspector/test/browser_ruleview_multiple-properties-unfinished_01.js
@@ -31,19 +31,19 @@ function waitRuleViewChanged(view, n) {
       deferred.resolve();
     }
   }
   view.on("ruleview-changed", listener);
   return deferred.promise;
 }
 function* testCreateNewMultiUnfinished(inspector, ruleEditor, view) {
   let onMutation = inspector.once("markupmutation");
-  // There is 6 rule-view updates, one for the rule view creation,
-  // one for each new property and one last for throttle update.
-  let onRuleViewChanged = waitRuleViewChanged(view, 6);
+  // There is 5 rule-view updates, one for the rule view creation,
+  // one for each new property
+  let onRuleViewChanged = waitRuleViewChanged(view, 5);
   yield createNewRuleViewProperty(ruleEditor,
     "color:blue;background : orange   ; text-align:center; border-color: ");
   yield onMutation;
   yield onRuleViewChanged;
 
   is(ruleEditor.rule.textProps.length, 4, "Should have created new text properties.");
   is(ruleEditor.propertyList.children.length, 4, "Should have created property editors.");
 
--- a/browser/devtools/styleinspector/test/head.js
+++ b/browser/devtools/styleinspector/test/head.js
@@ -409,39 +409,25 @@ function* waitForComputedStyleProperty(s
 }
 
 /**
  * Given an inplace editable element, click to switch it to edit mode, wait for
  * focus
  * @return a promise that resolves to the inplace-editor element when ready
  */
 let focusEditableField = Task.async(function*(ruleView, editable, xOffset=1, yOffset=1, options={}) {
-  // Focusing the name or value input is going to fire a preview and update the rule view
-  let expectRuleViewUpdate =
-    editable.classList.contains("ruleview-propertyname") ||
-    editable.classList.contains("ruleview-propertyvalue");
-  let onRuleViewChanged;
-  if (expectRuleViewUpdate) {
-    onRuleViewChanged = ruleView.once("ruleview-changed");
-  }
-
   let onFocus = once(editable.parentNode, "focus", true);
   info("Clicking on editable field to turn to edit mode");
   EventUtils.synthesizeMouse(editable, xOffset, yOffset, options,
     editable.ownerDocument.defaultView);
-  let event = yield onFocus;
+  yield onFocus;
 
   info("Editable field gained focus, returning the input field now");
   let onEdit = inplaceEditor(editable.ownerDocument.activeElement);
 
-  if (expectRuleViewUpdate) {
-    info("Waiting for rule view update");
-    yield onRuleViewChanged;
-  }
-
   return onEdit;
 });
 
 /**
  * Given a tooltip object instance (see Tooltip.js), checks if it is set to
  * toggle and hover and if so, checks if the given target is a valid hover target.
  * This won't actually show the tooltip (the less we interact with XUL panels
  * during test runs, the better).