Bug 1293616 - insert new rule in correct position in rule list; r?pbro
MozReview-Commit-ID: C4ib7ooI8BX
--- a/devtools/client/inspector/rules/test/browser.ini
+++ b/devtools/client/inspector/rules/test/browser.ini
@@ -123,16 +123,18 @@ skip-if = os == "mac" # Bug 1245996 : cl
[browser_rules_edit-selector_02.js]
[browser_rules_edit-selector_03.js]
[browser_rules_edit-selector_04.js]
[browser_rules_edit-selector_05.js]
[browser_rules_edit-selector_06.js]
[browser_rules_edit-selector_07.js]
[browser_rules_edit-selector_08.js]
[browser_rules_edit-selector_09.js]
+[browser_rules_edit-selector_10.js]
+[browser_rules_edit-selector_11.js]
[browser_rules_edit-value-after-name_01.js]
[browser_rules_edit-value-after-name_02.js]
[browser_rules_edit-value-after-name_03.js]
[browser_rules_edit-value-after-name_04.js]
[browser_rules_editable-field-focus_01.js]
[browser_rules_editable-field-focus_02.js]
[browser_rules_eyedropper.js]
[browser_rules_filtereditor-appears-on-swatch-click.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/rules/test/browser_rules_edit-selector_10.js
@@ -0,0 +1,64 @@
+/* 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";
+
+// Regression test for bug 1293616: make sure that editing a selector
+// keeps the rule in the proper position.
+
+const TEST_URI = `
+ <style type="text/css">
+ #testid span, #testid p {
+ background: aqua;
+ }
+ span {
+ background: fuchsia;
+ }
+ </style>
+ <div id="testid">
+ <span class="pickme">
+ Styled Node
+ </span>
+ </div>
+`;
+
+add_task(function* () {
+ yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+ let {inspector, view} = yield openRuleView();
+ yield selectNode(".pickme", inspector);
+ yield testEditSelector(view);
+});
+
+function* testEditSelector(view) {
+ let ruleEditor = getRuleViewRuleEditor(view, 1);
+ let editor = yield focusEditableField(view, ruleEditor.selectorText);
+
+ editor.input.value = "#testid span";
+ let onRuleViewChanged = once(view, "ruleview-changed");
+ EventUtils.synthesizeKey("VK_RETURN", {});
+ yield onRuleViewChanged;
+
+ // Escape the new property editor after editing the selector
+ let onBlur = once(view.styleDocument.activeElement, "blur");
+ EventUtils.synthesizeKey("VK_ESCAPE", {}, view.styleWindow);
+ yield onBlur;
+
+ // Get the new rule editor that replaced the original
+ ruleEditor = getRuleViewRuleEditor(view, 1);
+
+ info("Check that the correct rules are visible");
+ is(view._elementStyle.rules.length, 3, "Should have 3 rules.");
+ is(ruleEditor.element.getAttribute("unmatched"), "false", "Rule editor is matched.");
+
+ let props = ruleEditor.rule.textProps;
+ is(props.length, 1, "Rule has correct number of properties");
+ is(props[0].name, "background", "Found background property");
+ ok(!props[0].overridden, "Background property is not overridden");
+
+ ruleEditor = getRuleViewRuleEditor(view, 2);
+ props = ruleEditor.rule.textProps;
+ is(props.length, 1, "Rule has correct number of properties");
+ is(props[0].name, "background", "Found background property");
+ ok(props[0].overridden, "Background property is overridden");
+}
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/rules/test/browser_rules_edit-selector_11.js
@@ -0,0 +1,69 @@
+/* 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";
+
+// Regression test for bug 1293616, where editing a selector should
+// change the relative priority of the rule.
+
+const TEST_URI = `
+ <style type="text/css">
+ #testid {
+ background: aqua;
+ }
+ .pickme {
+ background: seagreen;
+ }
+ span {
+ background: fuchsia;
+ }
+ </style>
+ <div>
+ <span id="testid" class="pickme">
+ Styled Node
+ </span>
+ </div>
+`;
+
+add_task(function* () {
+ yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+ let {inspector, view} = yield openRuleView();
+ yield selectNode(".pickme", inspector);
+ yield testEditSelector(view);
+});
+
+function* testEditSelector(view) {
+ let ruleEditor = getRuleViewRuleEditor(view, 1);
+ let editor = yield focusEditableField(view, ruleEditor.selectorText);
+
+ editor.input.value = ".pickme";
+ let onRuleViewChanged = once(view, "ruleview-changed");
+ EventUtils.synthesizeKey("VK_RETURN", {});
+ yield onRuleViewChanged;
+
+ // Escape the new property editor after editing the selector
+ let onBlur = once(view.styleDocument.activeElement, "blur");
+ EventUtils.synthesizeKey("VK_ESCAPE", {}, view.styleWindow);
+ yield onBlur;
+
+ // Get the new rule editor that replaced the original
+ ruleEditor = getRuleViewRuleEditor(view, 1);
+
+ info("Check that the correct rules are visible");
+ is(view._elementStyle.rules.length, 4, "Should have 4 rules.");
+ is(ruleEditor.element.getAttribute("unmatched"), "false", "Rule editor is matched.");
+
+ let props = ruleEditor.rule.textProps;
+ is(props.length, 1, "Rule has correct number of properties");
+ is(props[0].name, "background", "Found background property");
+ is(props[0].value, "aqua", "Background property is aqua");
+ ok(props[0].overridden, "Background property is overridden");
+
+ ruleEditor = getRuleViewRuleEditor(view, 2);
+ props = ruleEditor.rule.textProps;
+ is(props.length, 1, "Rule has correct number of properties");
+ is(props[0].name, "background", "Found background property");
+ is(props[0].value, "seagreen", "Background property is seagreen");
+ ok(!props[0].overridden, "Background property is not overridden");
+}
--- a/devtools/client/inspector/rules/views/rule-editor.js
+++ b/devtools/client/inspector/rules/views/rule-editor.js
@@ -22,16 +22,17 @@ const {
parsePseudoClassesAndAttributes,
SELECTOR_ATTRIBUTE,
SELECTOR_ELEMENT,
SELECTOR_PSEUDO_CLASS
} = require("devtools/shared/css/parsing-utils");
const promise = require("promise");
const Services = require("Services");
const EventEmitter = require("devtools/shared/event-emitter");
+const {Task} = require("devtools/shared/task");
const STYLE_INSPECTOR_PROPERTIES = "devtools-shared/locale/styleinspector.properties";
const {LocalizationHelper} = require("devtools/shared/l10n");
const STYLE_INSPECTOR_L10N = new LocalizationHelper(STYLE_INSPECTOR_PROPERTIES);
const HTML_NS = "http://www.w3.org/1999/xhtml";
const XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
@@ -512,72 +513,98 @@ RuleEditor.prototype = {
*
* @param {String} value
* The value contained in the editor.
* @param {Boolean} commit
* True if the change should be applied.
* @param {Number} direction
* The move focus direction number.
*/
- _onSelectorDone: function (value, commit, direction) {
+ _onSelectorDone: Task.async(function* (value, commit, direction) {
if (!commit || this.isEditing || value === "" ||
value === this.rule.selectorText) {
return;
}
let ruleView = this.ruleView;
let elementStyle = ruleView._elementStyle;
let element = elementStyle.element;
let supportsUnmatchedRules =
this.rule.domRule.supportsModifySelectorUnmatched;
this.isEditing = true;
- this.rule.domRule.modifySelector(element, value).then(response => {
- this.isEditing = false;
+ try {
+ let response = yield this.rule.domRule.modifySelector(element, value);
if (!supportsUnmatchedRules) {
+ this.isEditing = false;
+
if (response) {
this.ruleView.refreshPanel();
}
return;
}
+ // We recompute the list of applied styles, because editing a
+ // selector might cause this rule's position to change.
+ let applied = yield elementStyle.pageStyle.getApplied(element, {
+ inherited: true,
+ matchedSelectors: true,
+ filter: elementStyle.showUserAgentStyles ? "ua" : undefined
+ });
+
+ this.isEditing = false;
+
let {ruleProps, isMatching} = response;
if (!ruleProps) {
// Notify for changes, even when nothing changes,
// just to allow tests being able to track end of this request.
ruleView.emit("ruleview-invalid-selector");
return;
}
ruleProps.isUnmatched = !isMatching;
let newRule = new Rule(elementStyle, ruleProps);
let editor = new RuleEditor(ruleView, newRule);
let rules = elementStyle.rules;
- rules.splice(rules.indexOf(this.rule), 1);
- rules.push(newRule);
+ let newRuleIndex = applied.findIndex((r) => r.rule == ruleProps.rule);
+ let oldIndex = rules.indexOf(this.rule);
+
+ // If the selector no longer matches, then we leave the rule in
+ // the same relative position.
+ if (newRuleIndex === -1) {
+ newRuleIndex = oldIndex;
+ }
+
+ // Remove the old rule and insert the new rule.
+ rules.splice(oldIndex, 1);
+ rules.splice(newRuleIndex, 0, newRule);
elementStyle._changed();
elementStyle.markOverriddenAll();
+ // We install the new editor in place of the old -- you might
+ // think we would replicate the list-modification logic above,
+ // but that is complicated due to the way the UI installs
+ // pseudo-element rules and the like.
this.element.parentNode.replaceChild(editor.element, this.element);
// Remove highlight for modified selector
if (ruleView.highlightedSelector) {
ruleView.toggleSelectorHighlighter(ruleView.lastSelectorIcon,
ruleView.highlightedSelector);
}
editor._moveSelectorFocus(direction);
- }).then(null, err => {
+ } catch (err) {
this.isEditing = false;
promiseWarn(err);
- });
- },
+ }
+ }),
/**
* Handle moving the focus change after a tab or return keypress in the
* selector inplace editor.
*
* @param {Number} direction
* The move focus direction number.
*/