Bug 1248274 - Fix click on augmented value elements (swatch, url,…) on the RuleView when property's name is being edited. r=pbro
Do not update the value span if the property name has not been modified.
Add tests to ensure the bug is fixed.
MozReview-Commit-ID: 7AHnd4iFmYW
--- a/devtools/client/inspector/rules/test/browser.ini
+++ b/devtools/client/inspector/rules/test/browser.ini
@@ -109,16 +109,17 @@ subsuite = clipboard
[browser_rules_edit-property_02.js]
[browser_rules_edit-property_03.js]
[browser_rules_edit-property_04.js]
[browser_rules_edit-property_05.js]
[browser_rules_edit-property_06.js]
[browser_rules_edit-property_07.js]
[browser_rules_edit-property_08.js]
[browser_rules_edit-property_09.js]
+[browser_rules_edit-property_10.js]
[browser_rules_edit-selector-click.js]
[browser_rules_edit-selector-click-on-scrollbar.js]
skip-if = os == "mac" # Bug 1245996 : click on scrollbar not working on OSX
[browser_rules_edit-selector-commit.js]
[browser_rules_edit-selector_01.js]
[browser_rules_edit-selector_02.js]
[browser_rules_edit-selector_03.js]
[browser_rules_edit-selector_04.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/rules/test/browser_rules_edit-property_10.js
@@ -0,0 +1,145 @@
+/* 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 clicking on swatch-preceeded value while editing the property name
+// will result in editing the property value. See also Bug 1248274.
+
+const TEST_URI = `
+ <style type="text/css">
+ #testid {
+ color: red;
+ background: linear-gradient(
+ 90deg,
+ rgb(183,222,237),
+ rgb(33,180,226),
+ rgb(31,170,217),
+ rgba(200,170,140,0.5));
+ }
+ </style>
+ <div id="testid">Styled Node</div>
+`;
+
+add_task(function* () {
+ yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+ let {inspector, view} = yield openRuleView();
+
+ yield selectNode("#testid", inspector);
+
+ let ruleEditor = getRuleViewRuleEditor(view, 1);
+ yield testColorValueSpanClick(ruleEditor, view);
+ yield testColorSwatchShiftClick(ruleEditor, view);
+ yield testColorSwatchClick(ruleEditor, view);
+});
+
+function* testColorValueSpanClick(ruleEditor, view) {
+ info("Test click on color span while editing property name");
+ let propEditor = ruleEditor.rule.textProps[0].editor;
+ let colorSpan = propEditor.valueSpan.querySelector(".ruleview-color");
+
+ info("Focus the color name span");
+ yield focusEditableField(view, propEditor.nameSpan);
+ let editor = inplaceEditor(propEditor.doc.activeElement);
+
+ info("Modify the property to border-color to trigger the " +
+ "property-value-updated event");
+ editor.input.value = "border-color";
+
+ let onPropertyValueUpdate = view.once("property-value-updated");
+
+ info("blur propEditor.nameSpan by clicking on the color span");
+ EventUtils.synthesizeMouse(colorSpan, 1, 1, {}, propEditor.doc.defaultView);
+
+ info("wait for the property value to be updated");
+ yield onPropertyValueUpdate;
+
+ editor = inplaceEditor(propEditor.doc.activeElement);
+ is(inplaceEditor(propEditor.valueSpan), editor,
+ "The property value editor got focused");
+
+ info("blur valueSpan editor to trigger ruleview-changed event and prevent " +
+ "having pending request");
+ let onRuleViewChanged = view.once("ruleview-changed");
+ editor.input.blur();
+ yield onRuleViewChanged;
+}
+
+function* testColorSwatchShiftClick(ruleEditor, view) {
+ info("Test shift + click on color swatch while editing property name");
+ let propEditor = ruleEditor.rule.textProps[1].editor;
+ let swatchSpan = propEditor.valueSpan.querySelectorAll(
+ ".ruleview-colorswatch")[2];
+
+ info("Focus the background name span");
+ yield focusEditableField(view, propEditor.nameSpan);
+ let editor = inplaceEditor(propEditor.doc.activeElement);
+
+ info("Modify the property to background-image to trigger the " +
+ "property-value-updated event");
+ editor.input.value = "background-image";
+
+ let onPropertyValueUpdate = view.once("property-value-updated");
+ let onSwatchUnitChange = swatchSpan.once("unit-change");
+ let onRuleViewChanged = view.once("ruleview-changed");
+
+ info("blur propEditor.nameSpan by clicking on the color swatch");
+ EventUtils.synthesizeMouseAtCenter(swatchSpan, {shiftKey: true},
+ propEditor.doc.defaultView);
+
+ info("wait for ruleview-changed event to be triggered to prevent pending " +
+ "requests");
+ yield onRuleViewChanged;
+
+ info("wait for the color unit to change");
+ yield onSwatchUnitChange;
+ ok(true, "the color unit was changed");
+
+ info("wait for the property value to be updated");
+ yield onPropertyValueUpdate;
+
+ ok(!inplaceEditor(propEditor.valueSpan), "The inplace editor wasn't shown " +
+ "as a result of the color swatch shift + click");
+}
+
+function* testColorSwatchClick(ruleEditor, view) {
+ info("Test click on color swatch while editing property name");
+ let propEditor = ruleEditor.rule.textProps[1].editor;
+ let swatchSpan = propEditor.valueSpan.querySelectorAll(
+ ".ruleview-colorswatch")[3];
+ let colorPicker = view.tooltips.colorPicker;
+
+ info("Focus the background-image name span");
+ yield focusEditableField(view, propEditor.nameSpan);
+ let editor = inplaceEditor(propEditor.doc.activeElement);
+
+ info("Modify the property to background to trigger the " +
+ "property-value-updated event");
+ editor.input.value = "background";
+
+ let onRuleViewChanged = view.once("ruleview-changed");
+ let onPropertyValueUpdate = view.once("property-value-updated");
+ let onShown = colorPicker.tooltip.once("shown");
+
+ info("blur propEditor.nameSpan by clicking on the color swatch");
+ EventUtils.synthesizeMouseAtCenter(swatchSpan, {},
+ propEditor.doc.defaultView);
+
+ info("wait for ruleview-changed event to be triggered to prevent pending " +
+ "requests");
+ yield onRuleViewChanged;
+
+ info("wait for the property value to be updated");
+ yield onPropertyValueUpdate;
+
+ info("wait for the color picker to be shown");
+ yield onShown;
+
+ ok(true, "The color picker was shown on click of the color swatch");
+ ok(!inplaceEditor(propEditor.valueSpan),
+ "The inplace editor wasn't shown as a result of the color swatch click");
+
+ let spectrum = yield colorPicker.spectrum;
+ is(spectrum.rgb, "200,170,140,0.5", "The correct color picker was shown");
+}
--- a/devtools/client/inspector/rules/views/text-property-editor.js
+++ b/devtools/client/inspector/rules/views/text-property-editor.js
@@ -18,16 +18,34 @@ const {
} = require("devtools/client/inspector/shared/utils");
const {
parseDeclarations,
parseSingleValue,
} = require("devtools/shared/css-parsing-utils");
const HTML_NS = "http://www.w3.org/1999/xhtml";
+const SHARED_SWATCH_CLASS = "ruleview-swatch";
+const COLOR_SWATCH_CLASS = "ruleview-colorswatch";
+const BEZIER_SWATCH_CLASS = "ruleview-bezierswatch";
+const FILTER_SWATCH_CLASS = "ruleview-filterswatch";
+const ANGLE_SWATCH_CLASS = "ruleview-angleswatch";
+
+/*
+ * An actionable element is an element which on click triggers a specific action
+ * (e.g. shows a color tooltip, opens a link, …).
+ */
+const ACTIONABLE_ELEMENTS_SELECTORS = [
+ `.${COLOR_SWATCH_CLASS}`,
+ `.${BEZIER_SWATCH_CLASS}`,
+ `.${FILTER_SWATCH_CLASS}`,
+ `.${ANGLE_SWATCH_CLASS}`,
+ "a"
+];
+
/**
* TextPropertyEditor is responsible for the following:
* Owns a TextProperty object.
* Manages changes to the TextProperty.
* Can be expanded to display computed properties.
* Can mark a property disabled or enabled.
*
* @param {RuleEditor} ruleEditor
@@ -39,30 +57,33 @@ function TextPropertyEditor(ruleEditor,
this.ruleEditor = ruleEditor;
this.ruleView = this.ruleEditor.ruleView;
this.doc = this.ruleEditor.doc;
this.popup = this.ruleView.popup;
this.prop = property;
this.prop.editor = this;
this.browserWindow = this.doc.defaultView.top;
this._populatedComputed = false;
+ this._hasPendingClick = false;
+ this._clickedElementOptions = null;
const toolbox = this.ruleView.inspector.toolbox;
this.cssProperties = getCssProperties(toolbox);
this._onEnableClicked = this._onEnableClicked.bind(this);
this._onExpandClicked = this._onExpandClicked.bind(this);
this._onStartEditing = this._onStartEditing.bind(this);
this._onNameDone = this._onNameDone.bind(this);
this._onValueDone = this._onValueDone.bind(this);
this._onSwatchCommit = this._onSwatchCommit.bind(this);
this._onSwatchPreview = this._onSwatchPreview.bind(this);
this._onSwatchRevert = this._onSwatchRevert.bind(this);
this._onValidate = throttle(this._previewValue, 10, this);
this.update = this.update.bind(this);
+ this.updatePropertyState = this.updatePropertyState.bind(this);
this._create();
this.update();
}
TextPropertyEditor.prototype = {
/**
* Boolean indicating if the name or value is being currently edited.
@@ -190,17 +211,17 @@ TextPropertyEditor.prototype = {
this.nameSpan.click();
}
}, false);
editableField({
start: this._onStartEditing,
element: this.nameSpan,
done: this._onNameDone,
- destroy: this.update,
+ destroy: this.updatePropertyState,
advanceChars: ":",
contentType: InplaceEditor.CONTENT_TYPES.CSS_PROPERTY,
popup: this.popup
});
// Auto blur name field on multiple CSS rules get pasted in.
this.nameContainer.addEventListener("paste",
blurOnMultipleProperties(this.cssProperties), false);
@@ -210,16 +231,47 @@ TextPropertyEditor.prototype = {
event.stopPropagation();
// Forward clicks on valueContainer to the editable valueSpan
if (event.target === this.valueContainer) {
this.valueSpan.click();
}
}, false);
+ // The mousedown event could trigger a blur event on nameContainer, which
+ // will trigger a call to the update function. The update function clears
+ // valueSpan's markup. Thus the regular click event does not bubble up, and
+ // listener's callbacks are not called.
+ // So we need to remember where the user clicks in order to re-trigger the click
+ // after the valueSpan's markup is re-populated. We only need to track this for
+ // valueSpan's child elements, because direct click on valueSpan will always
+ // trigger a click event.
+ this.valueSpan.addEventListener("mousedown", (event) => {
+ let clickedEl = event.target;
+ if (clickedEl === this.valueSpan) {
+ return;
+ }
+ this._hasPendingClick = true;
+
+ let matchedSelector = ACTIONABLE_ELEMENTS_SELECTORS.find(
+ (selector) => clickedEl.matches(selector));
+ if (matchedSelector) {
+ let similarElements = [...this.valueSpan.querySelectorAll(matchedSelector)];
+ this._clickedElementOptions = {
+ selector: matchedSelector,
+ index: similarElements.indexOf(clickedEl)
+ };
+ }
+ }, false);
+
+ this.valueSpan.addEventListener("mouseup", (event) => {
+ this._clickedElementOptions = null;
+ this._hasPendingClick = false;
+ }, false);
+
this.valueSpan.addEventListener("click", (event) => {
let target = event.target;
if (target.nodeName === "a") {
event.stopPropagation();
event.preventDefault();
this.browserWindow.openUILinkIn(target.href, "tab");
}
@@ -258,37 +310,17 @@ TextPropertyEditor.prototype = {
/**
* Populate the span based on changes to the TextProperty.
*/
update: function () {
if (this.ruleView.isDestroyed) {
return;
}
- if (this.prop.enabled) {
- this.enable.style.removeProperty("visibility");
- this.enable.setAttribute("checked", "");
- } else {
- this.enable.style.visibility = "visible";
- this.enable.removeAttribute("checked");
- }
-
- this.warning.hidden = this.editing || this.isValid();
- this.filterProperty.hidden = this.editing ||
- !this.isValid() ||
- !this.prop.overridden ||
- this.ruleEditor.rule.isUnmatched;
-
- if (!this.editing &&
- (this.prop.overridden || !this.prop.enabled ||
- !this.prop.isKnownProperty())) {
- this.element.classList.add("ruleview-overridden");
- } else {
- this.element.classList.remove("ruleview-overridden");
- }
+ this.updatePropertyState();
let name = this.prop.name;
this.nameSpan.textContent = name;
// Combine the property's value and priority into one string for
// the value.
let store = this.rule.elementStyle.store;
let val = store.userProperties.getProperty(this.rule.style, name,
@@ -300,43 +332,39 @@ TextPropertyEditor.prototype = {
let propDirty = store.userProperties.contains(this.rule.style, name);
if (propDirty) {
this.element.setAttribute("dirty", "");
} else {
this.element.removeAttribute("dirty");
}
- const sharedSwatchClass = "ruleview-swatch ";
- const colorSwatchClass = "ruleview-colorswatch";
- const bezierSwatchClass = "ruleview-bezierswatch";
- const filterSwatchClass = "ruleview-filterswatch";
- const angleSwatchClass = "ruleview-angleswatch";
-
let outputParser = this.ruleView._outputParser;
let parserOptions = {
- colorSwatchClass: sharedSwatchClass + colorSwatchClass,
+ colorSwatchClass: SHARED_SWATCH_CLASS + " " + COLOR_SWATCH_CLASS,
colorClass: "ruleview-color",
- bezierSwatchClass: sharedSwatchClass + bezierSwatchClass,
+ bezierSwatchClass: SHARED_SWATCH_CLASS + " " + BEZIER_SWATCH_CLASS,
bezierClass: "ruleview-bezier",
- filterSwatchClass: sharedSwatchClass + filterSwatchClass,
+ filterSwatchClass: SHARED_SWATCH_CLASS + " " + FILTER_SWATCH_CLASS,
filterClass: "ruleview-filter",
- angleSwatchClass: sharedSwatchClass + angleSwatchClass,
+ angleSwatchClass: SHARED_SWATCH_CLASS + " " + ANGLE_SWATCH_CLASS,
angleClass: "ruleview-angle",
defaultColorType: !propDirty,
urlClass: "theme-link",
baseURI: this.sheetHref
};
let frag = outputParser.parseCssProperty(name, val, parserOptions);
this.valueSpan.innerHTML = "";
this.valueSpan.appendChild(frag);
+ this.ruleView.emit("property-value-updated", this.valueSpan);
+
// Attach the color picker tooltip to the color swatches
this._colorSwatchSpans =
- this.valueSpan.querySelectorAll("." + colorSwatchClass);
+ this.valueSpan.querySelectorAll("." + COLOR_SWATCH_CLASS);
if (this.ruleEditor.isEditable) {
for (let span of this._colorSwatchSpans) {
// Adding this swatch to the list of swatches our colorpicker
// knows about
this.ruleView.tooltips.colorPicker.addSwatch(span, {
onShow: this._onStartEditing,
onPreview: this._onSwatchPreview,
onCommit: this._onSwatchCommit,
@@ -345,72 +373,120 @@ TextPropertyEditor.prototype = {
span.on("unit-change", this._onSwatchCommit);
let title = CssLogic.l10n("rule.colorSwatch.tooltip");
span.setAttribute("title", title);
}
}
// Attach the cubic-bezier tooltip to the bezier swatches
this._bezierSwatchSpans =
- this.valueSpan.querySelectorAll("." + bezierSwatchClass);
+ this.valueSpan.querySelectorAll("." + BEZIER_SWATCH_CLASS);
if (this.ruleEditor.isEditable) {
for (let span of this._bezierSwatchSpans) {
// Adding this swatch to the list of swatches our colorpicker
// knows about
this.ruleView.tooltips.cubicBezier.addSwatch(span, {
onShow: this._onStartEditing,
onPreview: this._onSwatchPreview,
onCommit: this._onSwatchCommit,
onRevert: this._onSwatchRevert
});
let title = CssLogic.l10n("rule.bezierSwatch.tooltip");
span.setAttribute("title", title);
}
}
// Attach the filter editor tooltip to the filter swatch
- let span = this.valueSpan.querySelector("." + filterSwatchClass);
+ let span = this.valueSpan.querySelector("." + FILTER_SWATCH_CLASS);
if (this.ruleEditor.isEditable) {
if (span) {
parserOptions.filterSwatch = true;
this.ruleView.tooltips.filterEditor.addSwatch(span, {
onShow: this._onStartEditing,
onPreview: this._onSwatchPreview,
onCommit: this._onSwatchCommit,
onRevert: this._onSwatchRevert
}, outputParser, parserOptions);
let title = CssLogic.l10n("rule.filterSwatch.tooltip");
span.setAttribute("title", title);
}
}
this.angleSwatchSpans =
- this.valueSpan.querySelectorAll("." + angleSwatchClass);
+ this.valueSpan.querySelectorAll("." + ANGLE_SWATCH_CLASS);
if (this.ruleEditor.isEditable) {
for (let angleSpan of this.angleSwatchSpans) {
angleSpan.on("unit-change", this._onSwatchCommit);
let title = CssLogic.l10n("rule.angleSwatch.tooltip");
angleSpan.setAttribute("title", title);
}
}
+ // Now that we have updated the property's value, we might have a pending
+ // click on the value container. If we do, we have to trigger a click event
+ // on the right element.
+ if (this._hasPendingClick) {
+ this._hasPendingClick = false;
+ let elToClick;
+
+ if (this._clickedElementOptions !== null) {
+ let {selector, index} = this._clickedElementOptions;
+ elToClick = this.valueSpan.querySelectorAll(selector)[index];
+
+ this._clickedElementOptions = null;
+ }
+
+ if (!elToClick) {
+ elToClick = this.valueSpan;
+ }
+ elToClick.click();
+ }
+
// Populate the computed styles.
this._updateComputed();
// Update the rule property highlight.
this.ruleView._updatePropertyHighlight(this);
},
_onStartEditing: function () {
this.element.classList.remove("ruleview-overridden");
this.enable.style.visibility = "hidden";
},
/**
+ * Update the visibility of the enable checkbox, the warning indicator and
+ * the filter property, as well as the overriden state of the property.
+ */
+ updatePropertyState: function () {
+ if (this.prop.enabled) {
+ this.enable.style.removeProperty("visibility");
+ this.enable.setAttribute("checked", "");
+ } else {
+ this.enable.style.visibility = "visible";
+ this.enable.removeAttribute("checked");
+ }
+
+ this.warning.hidden = this.editing || this.isValid();
+ this.filterProperty.hidden = this.editing ||
+ !this.isValid() ||
+ !this.prop.overridden ||
+ this.ruleEditor.rule.isUnmatched;
+
+ if (!this.editing &&
+ (this.prop.overridden || !this.prop.enabled ||
+ !this.prop.isKnownProperty())) {
+ this.element.classList.add("ruleview-overridden");
+ } else {
+ this.element.classList.remove("ruleview-overridden");
+ }
+ },
+
+ /**
* Update the indicator for computed styles. The computed styles themselves
* are populated on demand, when they become visible.
*/
_updateComputed: function () {
this.computed.innerHTML = "";
let showExpander = this.prop.computed.some(c => c.name !== this.prop.name);
this.expander.style.visibility = showExpander ? "visible" : "hidden";
--- a/devtools/client/shared/output-parser.js
+++ b/devtools/client/shared/output-parser.js
@@ -449,39 +449,38 @@ OutputParser.prototype = {
});
value.appendChild(nodes);
container.appendChild(value);
return container;
},
_onColorSwatchMouseDown: function (event) {
- // Prevent text selection in the case of shift-click or double-click.
- event.preventDefault();
-
if (!event.shiftKey) {
return;
}
+ // Prevent click event to be fired to not show the tooltip
+ event.stopPropagation();
+
let swatch = event.target;
let color = this.colorSwatches.get(swatch);
let val = color.nextColorUnit();
swatch.nextElementSibling.textContent = val;
swatch.emit("unit-change", val);
},
_onAngleSwatchMouseDown: function (event) {
- // Prevent text selection in the case of shift-click or double-click.
- event.preventDefault();
-
if (!event.shiftKey) {
return;
}
+ event.stopPropagation();
+
let swatch = event.target;
let angle = this.angleSwatches.get(swatch);
let val = angle.nextAngleUnit();
swatch.nextElementSibling.textContent = val;
swatch.emit("unit-change", val);
},