Bug 1499049 - (Part 9) Add tests for tracking changes to CSS declarations from the Rule view; r=pbro
authorRazvan Caliman <rcaliman@mozilla.com>
Thu, 25 Oct 2018 16:01:21 +0000
changeset 443109 d0c4e1db970ef48154e77ae540f8bc71b1431ed1
parent 443108 629deef4613c09a0f081fc804507355b250aab77
child 443110 5866c7a7e9066aa7451bf056cd82ca0683093bff
push id34939
push usernerli@mozilla.com
push dateFri, 26 Oct 2018 15:47:55 +0000
treeherdermozilla-central@760a16bf0d2b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1499049
milestone65.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 1499049 - (Part 9) Add tests for tracking changes to CSS declarations from the Rule view; r=pbro Depends on D9659 Adds infrastructure and tests for validating that changes to CSS declarations in the Inspector Rule view are tracked and show up in the Changes panel. Makes use of the `client/inspector/rules/test/head.js` file with helpers to instrument changes in the Rule view then observe results on the Changes panel. This is a first pass on tests. More will follow to validate: - declarations in at-rules; - nested at-rules; - declarations in various sources: inline styles, external stylesheets. Differential Revision: https://phabricator.services.mozilla.com/D9660
devtools/client/inspector/changes/ChangesView.js
devtools/client/inspector/changes/moz.build
devtools/client/inspector/changes/test/.eslintrc.js
devtools/client/inspector/changes/test/browser.ini
devtools/client/inspector/changes/test/browser_changes_declaration_disable.js
devtools/client/inspector/changes/test/browser_changes_declaration_edit_value.js
devtools/client/inspector/changes/test/browser_changes_declaration_remove.js
devtools/client/inspector/changes/test/head.js
devtools/client/inspector/inspector.js
devtools/client/inspector/test/shared-head.js
--- a/devtools/client/inspector/changes/ChangesView.js
+++ b/devtools/client/inspector/changes/ChangesView.js
@@ -12,17 +12,18 @@ const { Provider } = require("devtools/c
 const ChangesApp = createFactory(require("./components/ChangesApp"));
 
 const {
   resetChanges,
   trackChange,
 } = require("./actions/changes");
 
 class ChangesView {
-  constructor(inspector) {
+  constructor(inspector, window) {
+    this.document = window.document;
     this.inspector = inspector;
     this.store = this.inspector.store;
     this.toolbox = this.inspector.toolbox;
 
     this.onAddChange = this.onAddChange.bind(this);
     this.onClearChanges = this.onClearChanges.bind(this);
     this.destroy = this.destroy.bind(this);
 
@@ -79,15 +80,16 @@ class ChangesView {
    */
   destroy() {
     this.store.dispatch(resetChanges());
 
     this.changesFront.off("add-change", this.onAddChange);
     this.changesFront.off("clear-changes", this.onClearChanges);
     this.changesFront = null;
 
+    this.document = null;
     this.inspector = null;
     this.store = null;
     this.toolbox = null;
   }
 }
 
 module.exports = ChangesView;
--- a/devtools/client/inspector/changes/moz.build
+++ b/devtools/client/inspector/changes/moz.build
@@ -10,8 +10,10 @@ DIRS += [
     'reducers',
     'utils',
 ]
 
 DevToolsModules(
     'ChangesManager.js',
     'ChangesView.js',
 )
+
+BROWSER_CHROME_MANIFESTS += ['test/browser.ini']
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/changes/test/.eslintrc.js
@@ -0,0 +1,6 @@
+"use strict";
+
+module.exports = {
+  // Extend from the shared list of defined globals for mochitests.
+  "extends": "../../../../.eslintrc.mochitests.js",
+};
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/changes/test/browser.ini
@@ -0,0 +1,17 @@
+[DEFAULT]
+tags = devtools
+subsuite = devtools
+support-files =
+  head.js
+  !/devtools/client/inspector/test/head.js
+  !/devtools/client/inspector/test/shared-head.js
+  !/devtools/client/inspector/rules/test/head.js
+  !/devtools/client/shared/test/shared-head.js
+  !/devtools/client/shared/test/shared-redux-head.js
+  !/devtools/client/shared/test/telemetry-test-helpers.js
+  !/devtools/client/shared/test/test-actor.js
+  !/devtools/client/shared/test/test-actor-registry.js
+
+[browser_changes_declaration_disable.js]
+[browser_changes_declaration_edit_value.js]
+[browser_changes_declaration_remove.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/changes/test/browser_changes_declaration_disable.js
@@ -0,0 +1,47 @@
+/* vim: set 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 toggling a CSS declaration in the Rule view is tracked.
+
+const TEST_URI = `
+  <style type='text/css'>
+    div {
+      color: red;
+    }
+  </style>
+  <div></div>
+`;
+
+add_task(async function() {
+  await addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  const { inspector, view: ruleView } = await openRuleView();
+  const { document: doc, store } = selectChangesView(inspector);
+  const panel = doc.querySelector("#sidebar-panel-changes");
+
+  await selectNode("div", inspector);
+  const rule = getRuleViewRuleEditor(ruleView, 1).rule;
+  const prop = rule.textProps[0];
+
+  let onTrackChange = waitUntilAction(store, "TRACK_CHANGE");
+  info("Disable the first declaration");
+  await togglePropStatus(ruleView, prop);
+  info("Wait for change to be tracked");
+  await onTrackChange;
+
+  let removedDeclarations = panel.querySelectorAll(".diff-remove");
+  is(removedDeclarations.length, 1, "Only one declaration was tracked as removed");
+
+  onTrackChange = waitUntilAction(store, "TRACK_CHANGE");
+  info("Re-enable the first declaration");
+  await togglePropStatus(ruleView, prop);
+  info("Wait for change to be tracked");
+  await onTrackChange;
+
+  const addedDeclarations = panel.querySelectorAll(".diff-add");
+  removedDeclarations = panel.querySelectorAll(".diff-remove");
+  is(addedDeclarations.length, 0, "No declarations were tracked as added");
+  is(removedDeclarations.length, 0, "No declarations were tracked as removed");
+});
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/changes/test/browser_changes_declaration_edit_value.js
@@ -0,0 +1,95 @@
+/* vim: set 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 editing the value of a CSS declaration in the Rule view is tracked.
+
+const TEST_URI = `
+  <style type='text/css'>
+    div {
+      color: red;
+    }
+  </style>
+  <div></div>
+`;
+
+/*
+  This array will be iterated over in order and the `value` property will be used to
+  update the value of the first CSS declaration.
+  The `add` and `remove` objects hold the expected values of the tracked declarations
+  shown in the Changes panel. If `add` or `remove` are null, it means we don't expect
+  any corresponding tracked declaration to show up in the Changes panel.
+ */
+const VALUE_CHANGE_ITERATIONS = [
+  // No changes should be tracked if the value did not actually change.
+  {
+    value: "red",
+    add: null,
+    remove: null,
+  },
+  // Changing the priority flag "!important" should be tracked.
+  {
+    value: "red !important",
+    add: { value: "red !important" },
+    remove: { value: "red" },
+  },
+  // Repeated changes should still show the original value as the one removed.
+  {
+    value: "blue",
+    add: { value: "blue" },
+    remove: { value: "red" },
+  },
+  // Restoring the original value should clear tracked changes.
+  {
+    value: "red",
+    add: null,
+    remove: null,
+  },
+];
+
+add_task(async function() {
+  await addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  const { inspector, view: ruleView } = await openRuleView();
+  const { document: doc, store } = selectChangesView(inspector);
+  const panel = doc.querySelector("#sidebar-panel-changes");
+
+  await selectNode("div", inspector);
+  const rule = getRuleViewRuleEditor(ruleView, 1).rule;
+  const prop = rule.textProps[0];
+
+  let onTrackChange;
+  let removeValue;
+  let addValue;
+
+  for (const { value, add, remove } of VALUE_CHANGE_ITERATIONS) {
+    onTrackChange = waitUntilAction(store, "TRACK_CHANGE");
+
+    info(`Change the CSS declaration value to ${value}`);
+    await setProperty(ruleView, prop, value);
+    info("Wait for the change to be tracked");
+    await onTrackChange;
+
+    addValue = panel.querySelector(".diff-add .declaration-value");
+    removeValue = panel.querySelector(".diff-remove .declaration-value");
+
+    if (add) {
+      is(addValue.textContent, add.value,
+        `Added declaration has expected value: ${add.value}`);
+      is(panel.querySelectorAll(".diff-add").length, 1,
+        "Only one declaration was tracked as added.");
+    } else {
+      ok(!addValue, `Added declaration was cleared`);
+    }
+
+    if (remove) {
+      is(removeValue.textContent, remove.value,
+        `Removed declaration has expected value: ${remove.value}`);
+      is(panel.querySelectorAll(".diff-remove").length, 1,
+        "Only one declaration was tracked as removed.");
+    } else {
+      ok(!removeValue, `Removed declaration was cleared`);
+    }
+  }
+});
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/changes/test/browser_changes_declaration_remove.js
@@ -0,0 +1,38 @@
+/* vim: set 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 removing a CSS declaration from a rule in the Rule view is tracked.
+
+const TEST_URI = `
+  <style type='text/css'>
+    div {
+      color: red;
+    }
+  </style>
+  <div></div>
+`;
+
+add_task(async function() {
+  await addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  const { inspector, view: ruleView } = await openRuleView();
+  const { document: doc, store } = selectChangesView(inspector);
+  const panel = doc.querySelector("#sidebar-panel-changes");
+
+  await selectNode("div", inspector);
+  const rule = getRuleViewRuleEditor(ruleView, 1).rule;
+  const prop = rule.textProps[0];
+
+  const onTrackChange = waitUntilAction(store, "TRACK_CHANGE");
+  info("Remove the first declaration");
+  await removeProperty(ruleView, prop);
+  info("Wait for change to be tracked");
+  await onTrackChange;
+
+  const removeName = panel.querySelector(".diff-remove .declaration-name");
+  const removeValue = panel.querySelector(".diff-remove .declaration-value");
+  is(removeName.textContent, "color", "Correct declaration name was tracked as removed");
+  is(removeValue.textContent, "red", "Correct declaration value was tracked as removed");
+});
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/changes/test/head.js
@@ -0,0 +1,31 @@
+ /* vim: set ts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+/* eslint no-unused-vars: [2, {"vars": "local"}] */
+/* import-globals-from ../../test/head.js */
+/* import-globals-from ../../../inspector/rules/test/head.js */
+/* import-globals-from ../../../inspector/test/shared-head.js */
+/* import-globals-from ../../../shared/test/shared-redux-head.js */
+"use strict";
+
+// Load the Rule view's test/head.js to make use of its helpers.
+// It loads inspector/test/head.js which itself loads inspector/test/shared-head.js
+Services.scriptloader.loadSubScript(
+  "chrome://mochitests/content/browser/devtools/client/inspector/rules/test/head.js",
+  this);
+
+// Load the shared Redux helpers.
+Services.scriptloader.loadSubScript(
+  "chrome://mochitests/content/browser/devtools/client/shared/test/shared-redux-head.js",
+  this);
+
+// Ensure the Changes panel is enabled before running the tests.
+Services.prefs.setBoolPref("devtools.inspector.changes.enabled", true);
+// Ensure the three-pane mode is enabled before running the tests.
+Services.prefs.setBoolPref("devtools.inspector.three-pane-enabled", true);
+
+registerCleanupFunction(() => {
+  Services.prefs.clearUserPref("devtools.inspector.changes.enabled");
+  Services.prefs.clearUserPref("devtools.inspector.three-pane-enabled");
+});
--- a/devtools/client/inspector/inspector.js
+++ b/devtools/client/inspector/inspector.js
@@ -273,17 +273,17 @@ Inspector.prototype = {
       this.selection.setNodeFront(this._defaultNode, { reason: "inspector-open" });
     }
 
     if (Services.prefs.getBoolPref(TRACK_CHANGES_PREF)) {
       // Get the Changes front, then call a method on it, which will instantiate
       // the ChangesActor. We want the ChangesActor to be guaranteed available before
       // the user makes any changes.
       this.changesFront = this.toolbox.target.getFront("changes");
-      await this.changesFront.allChanges();
+      this.changesFront.allChanges();
     }
 
     // Setup the splitter before the sidebar is displayed so, we don't miss any events.
     this.setupSplitter();
 
     // We can display right panel with: tab bar, markup view and breadbrumb. Right after
     // the splitter set the right and left panel sizes, in order to avoid resizing it
     // during load of the inspector.
--- a/devtools/client/inspector/test/shared-head.js
+++ b/devtools/client/inspector/test/shared-head.js
@@ -116,16 +116,34 @@ function openComputedView() {
       inspector: data.inspector,
       testActor: data.testActor,
       view,
     };
   });
 }
 
 /**
+ * Open the toolbox, with the inspector tool visible, and the changes view
+ * sidebar tab selected.
+ *
+ * @return a promise that resolves when the inspector is ready and the changes
+ * view is visible and ready
+ */
+function openChangesView() {
+  return openInspectorSidebarTab("changesview").then(data => {
+    return {
+      toolbox: data.toolbox,
+      inspector: data.inspector,
+      testActor: data.testActor,
+      view: data.inspector.changesView,
+    };
+  });
+}
+
+/**
  * Open the toolbox, with the inspector tool visible, and the layout view
  * sidebar tab selected to display the box model view with properties.
  *
  * @return {Promise} a promise that resolves when the inspector is ready and the layout
  *         view is visible and ready.
  */
 function openLayoutView() {
   return openInspectorSidebarTab("layoutview").then(data => {
@@ -172,16 +190,28 @@ function selectRuleView(inspector) {
  * @return {CssComputedView} the computed view
  */
 function selectComputedView(inspector) {
   inspector.sidebar.select("computedview");
   return inspector.getPanel("computedview").computedView;
 }
 
 /**
+ * Select the changes view sidebar tab on an already opened inspector panel.
+ *
+ * @param {InspectorPanel} inspector
+ *        The opened inspector panel
+ * @return {ChangesView} the changes view
+ */
+function selectChangesView(inspector) {
+  inspector.sidebar.select("changesview");
+  return inspector.changesView;
+}
+
+/**
  * Select the layout view sidebar tab on an already opened inspector panel.
  *
  * @param  {InspectorPanel} inspector
  * @return {BoxModel} the box model
  */
 function selectLayoutView(inspector) {
   inspector.sidebar.select("layoutview");
   return inspector.getPanel("boxmodel");