Bug 1241527 - 1 - Fix some unhandled rejected promises in colorpicker, cubicbezier and cssfilter ruleview tests; r=gl
authorPatrick Brosset <pbrosset@mozilla.com>
Thu, 21 Jan 2016 17:35:04 +0100
changeset 281353 4632b46da8a647ba620ddc8f40889ee53706dd0b
parent 281352 a45569eb51d7bb63602a80802f98b450b77ae9d5
child 281354 5406de49fa300f4a39e1237e2095b1f92f43f16b
push id29935
push userphilringnalda@gmail.com
push dateSun, 24 Jan 2016 02:12:02 +0000
treeherdermozilla-central@a2e81822194a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1241527
milestone46.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 1241527 - 1 - Fix some unhandled rejected promises in colorpicker, cubicbezier and cssfilter ruleview tests; r=gl
devtools/client/inspector/rules/test/browser_rules_colorpicker-appears-on-swatch-click.js
devtools/client/inspector/rules/test/browser_rules_colorpicker-edit-gradient.js
devtools/client/inspector/rules/test/browser_rules_colorpicker-multiple-changes.js
devtools/client/inspector/rules/test/browser_rules_cubicbezier-appears-on-swatch-click.js
devtools/client/inspector/rules/test/browser_rules_cubicbezier-commit-on-ENTER.js
devtools/client/inspector/rules/test/browser_rules_filtereditor-appears-on-swatch-click.js
devtools/client/inspector/rules/test/browser_rules_filtereditor-commit-on-ENTER.js
devtools/client/inspector/rules/test/head.js
devtools/client/shared/widgets/FilterWidget.js
--- a/devtools/client/inspector/rules/test/browser_rules_colorpicker-appears-on-swatch-click.js
+++ b/devtools/client/inspector/rules/test/browser_rules_colorpicker-appears-on-swatch-click.js
@@ -45,12 +45,10 @@ function* testColorPickerAppearsOnColorS
   let onShown = cPicker.tooltip.once("shown");
   swatch.click();
   yield onShown;
 
   ok(true, "The color picker was shown on click of the color swatch");
   ok(!inplaceEditor(swatch.parentNode),
     "The inplace editor wasn't shown as a result of the color swatch click");
 
-  let onModifications = view.once("ruleview-changed");
-  cPicker.hide();
-  yield onModifications;
+  yield hideTooltipAndWaitForRuleviewChanged(cPicker, view);
 }
--- a/devtools/client/inspector/rules/test/browser_rules_colorpicker-edit-gradient.js
+++ b/devtools/client/inspector/rules/test/browser_rules_colorpicker-edit-gradient.js
@@ -67,12 +67,10 @@ function* testPickingNewColor(view) {
   is(swatchEl.style.backgroundColor, "rgb(1, 1, 1)",
     "The color swatch's background was updated");
   is(colorEl.textContent, "#010101", "The color text was updated");
   is(content.getComputedStyle(content.document.body).backgroundImage,
     "linear-gradient(to left, rgb(1, 1, 1) 25%, rgb(51, 51, 51) 95%, " +
       "rgb(0, 0, 0) 100%)",
     "The gradient has been updated correctly");
 
-  let onModified = view.once("ruleview-changed");
-  cPicker.hide();
-  yield onModified;
+  yield hideTooltipAndWaitForRuleviewChanged(cPicker, view);
 }
--- a/devtools/client/inspector/rules/test/browser_rules_colorpicker-multiple-changes.js
+++ b/devtools/client/inspector/rules/test/browser_rules_colorpicker-multiple-changes.js
@@ -85,21 +85,17 @@ function* testComplexMultipleColorChange
     yield simulateColorPickerChange(ruleView, picker, rgba, {
       element: content.document.body,
       name: "backgroundColor",
       value: computed
     });
   }
 
   info("Closing the color picker");
-  let onHidden = picker.tooltip.once("hidden");
-  let onModified = ruleView.once("ruleview-changed");
-  picker.tooltip.hide();
-  yield onHidden;
-  yield onModified;
+  yield hideTooltipAndWaitForRuleviewChanged(picker.tooltip, ruleView);
 }
 
 function* testOverriddenMultipleColorChanges(inspector, ruleView) {
   yield selectNode("p", inspector);
 
   info("Getting the <body> tag's color property");
   let swatch = getRuleViewProperty(ruleView, "body", "color").valueSpan
     .querySelector(".ruleview-colorswatch");
--- a/devtools/client/inspector/rules/test/browser_rules_cubicbezier-appears-on-swatch-click.js
+++ b/devtools/client/inspector/rules/test/browser_rules_cubicbezier-appears-on-swatch-click.js
@@ -61,10 +61,10 @@ function* testAppears(view, swatch) {
 
   let onShown = bezier.tooltip.once("shown");
   swatch.click();
   yield onShown;
 
   ok(true, "The cubic-bezier tooltip was shown on click of the cibuc swatch");
   ok(!inplaceEditor(swatch.parentNode),
     "The inplace editor wasn't shown as a result of the cibuc swatch click");
-  bezier.hide();
+  yield hideTooltipAndWaitForRuleviewChanged(bezier, view);
 }
--- a/devtools/client/inspector/rules/test/browser_rules_cubicbezier-commit-on-ENTER.js
+++ b/devtools/client/inspector/rules/test/browser_rules_cubicbezier-commit-on-ENTER.js
@@ -44,18 +44,20 @@ function* testPressingEnterCommitsChange
       .transitionTimingFunction === expected;
   }, "Waiting for the change to be previewed on the element");
 
   ok(getRuleViewProperty(ruleView, "body", "transition").valueSpan.textContent
     .indexOf("cubic-bezier(") !== -1,
     "The text of the timing-function was updated");
 
   info("Sending RETURN key within the tooltip document");
-  let onHidden = bezierTooltip.tooltip.once("hidden");
+  // Pressing RETURN ends up doing 2 rule-view updates, one for the preview and
+  // one for the commit when the tooltip closes.
+  let onRuleViewChanged = waitForNEvents(ruleView, "ruleview-changed", 2);
   EventUtils.sendKey("RETURN", widget.parent.ownerDocument.defaultView);
-  yield onHidden;
+  yield onRuleViewChanged;
 
   is(content.getComputedStyle(content.document.body).transitionTimingFunction,
     expected, "The element's timing-function was kept after RETURN");
   ok(getRuleViewProperty(ruleView, "body", "transition").valueSpan.textContent
     .indexOf("cubic-bezier(") !== -1,
     "The text of the timing-function was kept after RETURN");
 }
--- a/devtools/client/inspector/rules/test/browser_rules_filtereditor-appears-on-swatch-click.js
+++ b/devtools/client/inspector/rules/test/browser_rules_filtereditor-appears-on-swatch-click.js
@@ -12,23 +12,22 @@ add_task(function*() {
 
   let {view} = yield openRuleView();
 
   info("Getting the filter swatch element");
   let swatch = getRuleViewProperty(view, "body", "filter").valueSpan
     .querySelector(".ruleview-filterswatch");
 
   let filterTooltip = view.tooltips.filterEditor;
-  let onShow = filterTooltip.tooltip.once("shown");
+  // Clicking on a cssfilter swatch sets the current filter value in the tooltip
+  // which, in turn, makes the FilterWidget emit an "updated" event that causes
+  // the rule-view to refresh. So we must wait for the ruleview-changed event.
   let onRuleViewChanged = view.once("ruleview-changed");
   swatch.click();
-  yield onShow;
-  // Clicking on swatch runs a preview of the current value
-  // and updates the rule-view
   yield onRuleViewChanged;
 
   ok(true, "The shown event was emitted after clicking on swatch");
   ok(!inplaceEditor(swatch.parentNode),
   "The inplace editor wasn't shown as a result of the filter swatch click");
 
   yield filterTooltip.widget;
-  filterTooltip.hide();
+  yield hideTooltipAndWaitForRuleviewChanged(filterTooltip, view);
 });
--- a/devtools/client/inspector/rules/test/browser_rules_filtereditor-commit-on-ENTER.js
+++ b/devtools/client/inspector/rules/test/browser_rules_filtereditor-commit-on-ENTER.js
@@ -11,26 +11,33 @@ add_task(function*() {
   yield addTab(TEST_URL);
   let {view} = yield openRuleView();
 
   info("Getting the filter swatch element");
   let swatch = getRuleViewProperty(view, "body", "filter").valueSpan
     .querySelector(".ruleview-filterswatch");
 
   let filterTooltip = view.tooltips.filterEditor;
-  let onShow = filterTooltip.tooltip.once("shown");
+  // Clicking on a cssfilter swatch sets the current filter value in the tooltip
+  // which, in turn, makes the FilterWidget emit an "updated" event that causes
+  // the rule-view to refresh. So we must wait for the ruleview-changed event.
+  let onRuleViewChanged = view.once("ruleview-changed");
   swatch.click();
-  yield onShow;
+  yield onRuleViewChanged;
 
   let widget = yield filterTooltip.widget;
 
+  onRuleViewChanged = view.once("ruleview-changed");
   widget.setCssValue("blur(2px)");
   yield waitForComputedStyleProperty("body", null, "filter", "blur(2px)");
+  yield onRuleViewChanged;
 
   ok(true, "Changes previewed on the element");
 
+  onRuleViewChanged = view.once("ruleview-changed");
   info("Pressing RETURN to commit changes");
   EventUtils.sendKey("RETURN", widget.styleWindow);
+  yield onRuleViewChanged;
 
   const computed = content.getComputedStyle(content.document.body);
   is(computed.filter, "blur(2px)",
      "The elemenet's filter was kept after RETURN");
 });
--- a/devtools/client/inspector/rules/test/head.js
+++ b/devtools/client/inspector/rules/test/head.js
@@ -276,16 +276,31 @@ function assertHoverTooltipOn(tooltip, e
   return isHoverTooltipTarget(tooltip, element).then(() => {
     ok(true, "A tooltip is defined on hover of the given element");
   }, () => {
     ok(false, "No tooltip is defined on hover of the given element");
   });
 }
 
 /**
+ * When a tooltip is closed, this ends up "commiting" the value changed within
+ * the tooltip (e.g. the color in case of a colorpicker) which, in turn, ends up
+ * setting the value of the corresponding css property in the rule-view.
+ * Use this function to close the tooltip and make sure the test waits for the
+ * ruleview-changed event.
+ * @param {Tooltip} tooltip
+ * @param {CSSRuleView} view
+ */
+function* hideTooltipAndWaitForRuleviewChanged(tooltip, view) {
+  let onModified = view.once("ruleview-changed");
+  tooltip.hide();
+  yield onModified;
+}
+
+/**
  * Listen for a new tab to open and return a promise that resolves when one
  * does and completes the load event.
  *
  * @return a promise that resolves to the tab object
  */
 var waitForTab = Task.async(function*() {
   info("Waiting for a tab to open");
   yield once(gBrowser.tabContainer, "TabOpen");
--- a/devtools/client/shared/widgets/FilterWidget.js
+++ b/devtools/client/shared/widgets/FilterWidget.js
@@ -720,17 +720,17 @@ CSSFilterEditorWidget.prototype = {
       // If the specified value is invalid, replace it with the
       // default.
       if (name !== "url") {
         if (!DOMUtils.cssPropertyIsValid("filter", name + "(" + value + ")")) {
           value = null;
         }
       }
 
-      this.add(name, value, quote);
+      this.add(name, value, quote, true);
     }
 
     this.emit("updated", this.getCssValue());
     this.render();
   },
 
   /**
     * Creates a new [name] filter record with value
@@ -740,18 +740,22 @@ CSSFilterEditorWidget.prototype = {
     * @param {String} value
     *        value of the filter (e.g. 30px, 20%)
     *        If this is |null|, then a default value may be supplied.
     * @param {String} quote
     *        For a url filter, the quoting style.  This can be a
     *        single quote, a double quote, or empty.
     * @return {Number}
     *        The index of the new filter in the current list of filters
+    * @param {Boolean}
+    *        By default, adding a new filter emits an "updated" event, but if
+    *        you're calling add in a loop and wait to emit a single event after
+    *        the loop yourself, set this parameter to true.
     */
-  add: function(name, value, quote) {
+  add: function(name, value, quote, noEvent) {
     const def = this._definition(name);
     if (!def) {
       return false;
     }
 
     if (value === null) {
       // UNIT_MAPPING[string] is an empty string (falsy), so
       // using || doesn't work here
@@ -789,17 +793,19 @@ CSSFilterEditorWidget.prototype = {
       if (value < min) {
         value = min;
       } else if (value > max) {
         value = max;
       }
     }
 
     const index = this.filters.push({value, unit, name, quote}) - 1;
-    this.emit("updated", this.getCssValue());
+    if (!noEvent) {
+      this.emit("updated", this.getCssValue());
+    }
 
     return index;
   },
 
   /**
     * returns value + unit of the specified filter
     *
     * @param {Number} index