Bug 1593944 - Test to ensure inactive CSS state does not linger when dependencies change. r=miker,pbro
authorRazvan Caliman <rcaliman@mozilla.com>
Wed, 13 Nov 2019 13:56:50 +0000
changeset 501739 b8a3793ecceb864e30b0c7d060dc9397770b1d9e
parent 501738 47a58584b81390485c7530b1eef7d59b2d6c32b2
child 501740 f11a1221a1adf18caae99adcfc4bd24a0fb44c72
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmiker, pbro
bugs1593944
milestone72.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 1593944 - Test to ensure inactive CSS state does not linger when dependencies change. r=miker,pbro Depends on D52560 - Adds a test to check that the steps for [Bug 1593944](https://bugzilla.mozilla.org/show_bug.cgi?id=1593944) no longer cause an issue. - Introduces a new `updateDeclaration()` helper in `devtools/client/inspector/rules/test/shared.js` to simplify updating property names and values in one step. - Updates `toggleDeclaration()` to remove unused `inspector` parameter; updates existing tests. Differential Revision: https://phabricator.services.mozilla.com/D52561
devtools/client/inspector/rules/test/browser.ini
devtools/client/inspector/rules/test/browser_rules_inactive_css_display-justify.js
devtools/client/inspector/rules/test/browser_rules_inactive_css_flexbox.js
devtools/client/inspector/rules/test/browser_rules_inactive_css_grid.js
devtools/client/inspector/rules/test/browser_rules_inactive_css_split-condition.js
devtools/client/inspector/rules/test/head.js
--- a/devtools/client/inspector/rules/test/browser.ini
+++ b/devtools/client/inspector/rules/test/browser.ini
@@ -204,16 +204,17 @@ skip-if = (os == "win" && debug) # bug 9
 [browser_rules_grid-toggle_05.js]
 [browser_rules_gridline-names-autocomplete.js]
 [browser_rules_gridline-names-are-shown-correctly.js]
 skip-if = os == 'linux' # focusEditableField times out consistently on linux.
 [browser_rules_guessIndentation.js]
 [browser_rules_highlight-element-rule.js]
 [browser_rules_highlight-property.js]
 [browser_rules_highlight-used-fonts.js]
+[browser_rules_inactive_css_display-justify.js]
 [browser_rules_inactive_css_flexbox.js]
 [browser_rules_inactive_css_grid.js]
 [browser_rules_inactive_css_inline.js]
 [browser_rules_inactive_css_split-condition.js]
 [browser_rules_inactive_css_visited.js]
 [browser_rules_inherited-properties_01.js]
 [browser_rules_inherited-properties_02.js]
 [browser_rules_inherited-properties_03.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/rules/test/browser_rules_inactive_css_display-justify.js
@@ -0,0 +1,47 @@
+/* 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";
+
+// Test that a declaration's inactive state doesn't linger on its previous state when
+// the declaration it depends on changes. Bug 1593944
+
+const TEST_URI = `
+<style>
+  div {
+    justify-content: center;
+    /*! display: flex */
+  }
+</style>
+<div>`;
+
+add_task(async function() {
+  await pushPref("devtools.inspector.inactive.css.enabled", true);
+  await addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  const { inspector, view } = await openRuleView();
+
+  await selectNode("div", inspector);
+
+  const justifyContent = { "justify-content": "center" };
+  const justifyItems = { "justify-items": "center" };
+  const displayFlex = { display: "flex" };
+  const displayGrid = { display: "grid" };
+
+  info("Enable display:flex and check that justify-content becomes active");
+  checkDeclarationIsInactive(view, 1, justifyContent);
+  await toggleDeclaration(view, 1, displayFlex);
+  checkDeclarationIsActive(view, 1, justifyContent);
+
+  info(
+    "Rename justify-content to justify-items and check that it becomes inactive"
+  );
+  await updateDeclaration(view, 1, justifyContent, justifyItems);
+  checkDeclarationIsInactive(view, 1, justifyItems);
+
+  info(
+    "Rename display:flex to display:grid and check that justify-items becomes active"
+  );
+  await updateDeclaration(view, 1, displayFlex, displayGrid);
+  checkDeclarationIsActive(view, 1, justifyItems);
+});
--- a/devtools/client/inspector/rules/test/browser_rules_inactive_css_flexbox.js
+++ b/devtools/client/inspector/rules/test/browser_rules_inactive_css_flexbox.js
@@ -149,13 +149,13 @@ add_task(async function() {
   await pushPref("devtools.inspector.inactive.css.enabled", true);
 
   await addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
   const { inspector, view } = await openRuleView();
 
   await runInactiveCSSTests(view, inspector, BEFORE);
 
   // Toggle `display:flex` to disabled.
-  await toggleDeclaration(inspector, view, 0, {
+  await toggleDeclaration(view, 0, {
     display: "flex",
   });
   await runInactiveCSSTests(view, inspector, AFTER);
 });
--- a/devtools/client/inspector/rules/test/browser_rules_inactive_css_grid.js
+++ b/devtools/client/inspector/rules/test/browser_rules_inactive_css_grid.js
@@ -167,14 +167,14 @@ add_task(async function() {
   await pushPref("devtools.inspector.inactive.css.enabled", true);
 
   await addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
   const { inspector, view } = await openRuleView();
 
   await runInactiveCSSTests(view, inspector, BEFORE);
 
   // Toggle `display:grid` to disabled.
-  await toggleDeclaration(inspector, view, 0, {
+  await toggleDeclaration(view, 0, {
     display: "grid",
   });
   await view.once("ruleview-refreshed");
   await runInactiveCSSTests(view, inspector, AFTER);
 });
--- a/devtools/client/inspector/rules/test/browser_rules_inactive_css_split-condition.js
+++ b/devtools/client/inspector/rules/test/browser_rules_inactive_css_split-condition.js
@@ -21,11 +21,11 @@ const TEST_URI = `
 add_task(async function() {
   await pushPref("devtools.inspector.inactive.css.enabled", true);
   await addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
   const { inspector, view } = await openRuleView();
 
   await selectNode("div", inspector);
 
   checkDeclarationIsActive(view, 1, { gap: "1em" });
-  await toggleDeclaration(inspector, view, 2, { display: "grid" });
+  await toggleDeclaration(view, 2, { display: "grid" });
   checkDeclarationIsInactive(view, 1, { gap: "1em" });
 });
--- a/devtools/client/inspector/rules/test/head.js
+++ b/devtools/client/inspector/rules/test/head.js
@@ -715,49 +715,107 @@ function getPropertiesForRuleIndex(view,
   }
 
   return declaration;
 }
 
 /**
  * Toggle a declaration disabled or enabled.
  *
- * @param {InspectorPanel} inspector
- *        The instance of InspectorPanel currently loaded in the toolbox.
  * @param {ruleView} view
  *        The rule-view instance
  * @param {Number} ruleIndex
  *        The index of the CSS rule where we can find the declaration to be
  *        toggled.
  * @param {Object} declaration
  *        An object representing the declaration e.g. { color: "red" }.
  */
-async function toggleDeclaration(inspector, view, ruleIndex, declaration) {
-  const ruleEditor = getRuleViewRuleEditor(view, ruleIndex);
+async function toggleDeclaration(view, ruleIndex, declaration) {
+  const textProp = getTextProperty(view, ruleIndex, declaration);
   const [[name, value]] = Object.entries(declaration);
-
-  let textProp = null;
-  for (const currProp of ruleEditor.rule.textProps) {
-    if (currProp.name === name && currProp.value === value) {
-      textProp = currProp;
-      break;
-    }
-  }
-
   const dec = `${name}:${value}`;
   ok(textProp, `Declaration "${dec}" found`);
 
   const newStatus = textProp.enabled ? "disabled" : "enabled";
   info(`Toggling declaration "${dec}" of rule ${ruleIndex} to ${newStatus}`);
 
   await togglePropStatus(view, textProp);
   info("Toggled successfully.");
 }
 
 /**
+ * Update a declaration from a CSS rule in the Rules view
+ * by changing its property name, property value or both.
+ *
+ * @param {RuleView} view
+ *        Instance of RuleView.
+ * @param {Number} ruleIndex
+ *        The index of the CSS rule where to find the declaration.
+ * @param {Object} declaration
+ *        An object representing the target declaration e.g. { color: red }.
+ * @param {Object} newDeclaration
+ *        An object representing the desired updated declaration e.g. { display: none }.
+ */
+async function updateDeclaration(
+  view,
+  ruleIndex,
+  declaration,
+  newDeclaration = {}
+) {
+  const textProp = getTextProperty(view, ruleIndex, declaration);
+  const [[name, value]] = Object.entries(declaration);
+  const [[newName, newValue]] = Object.entries(newDeclaration);
+
+  if (newName && name !== newName) {
+    info(
+      `Updating declaration ${name}:${value};
+      Changing ${name} to ${newName}`
+    );
+    await renameProperty(view, textProp, newName);
+  }
+
+  if (newValue && value !== newValue) {
+    info(
+      `Updating declaration ${name}:${value};
+      Changing ${value} to ${newValue}`
+    );
+    await setProperty(view, textProp, newValue);
+  }
+}
+
+/**
+ * Get the TextProperty instance corresponding to a CSS declaration
+ * from a CSS rule in the Rules view.
+ *
+ * @param  {RuleView} view
+ *         Instance of RuleView.
+ * @param  {Number} ruleIndex
+ *         The index of the CSS rule where to find the declaration.
+ * @param  {Object} declaration
+ *         An object representing the target declaration e.g. { color: red }.
+ *         The first TextProperty instance which matches will be returned.
+ * @return {TextProperty}
+ */
+function getTextProperty(view, ruleIndex, declaration) {
+  const ruleEditor = getRuleViewRuleEditor(view, ruleIndex);
+  const [[name, value]] = Object.entries(declaration);
+  const textProp = ruleEditor.rule.textProps.find(prop => {
+    return prop.name === name && prop.value === value;
+  });
+
+  if (!textProp) {
+    throw Error(
+      `Declaration ${name}:${value} not found on rule at index ${ruleIndex}`
+    );
+  }
+
+  return textProp;
+}
+
+/**
  * Check that a declaration is marked inactive and that it has the expected
  * warning.
  *
  * @param {ruleView} view
  *        The rule-view instance.
  * @param {Number} ruleIndex
  *        The index we expect the rule to have in the rule-view.
  * @param {Object} declaration