Bug 1503896 - (Part 2) Support tracking multiple CSS declarations in one change; r=pbro
authorRazvan Caliman <rcaliman@mozilla.com>
Mon, 05 Nov 2018 14:50:04 +0000
changeset 444824 33c358d0d2c2e10a9fd67c7a303ec2e2c9425a5e
parent 444823 1601672ee6af41c7b233d469bcf1427b406261f2
child 444825 95ac332b63edbb9d3ea05f08b82c192aad35a1e8
push id35003
push userncsoregi@mozilla.com
push dateWed, 07 Nov 2018 16:16:52 +0000
treeherdermozilla-central@4407a6d3e374 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1503896
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 1503896 - (Part 2) Support tracking multiple CSS declarations in one change; r=pbro Depends on D10582 Improves the reducer for tracking CSS changes to handle more than one CSS declaration changed in one operation. This is a requirement for tracking whole rule removal or whole rule addition, like it happens when renaming a CSS selector in the Rules view. MozReview-Commit-ID: 25pf2GRiH4D Differential Revision: https://phabricator.services.mozilla.com/D10584
devtools/client/inspector/changes/reducers/changes.js
devtools/server/actors/styles.js
--- a/devtools/client/inspector/changes/reducers/changes.js
+++ b/devtools/client/inspector/changes/reducers/changes.js
@@ -164,56 +164,60 @@ const reducers = {
 
     change = { ...defaults, ...change };
     state = cloneState(state);
 
     const { type, href, index } = change.source;
     const { selector, ancestors, ruleIndex } = change;
     const sourceId = getSourceHash(change.source);
     const ruleId = getRuleHash({ selector, ancestors, ruleIndex });
+    // Type coerce to boolean: `false` if value is undefined or `null`, `true` otherwise.
+    const hasAdd = !!change.add;
+    const hasRemove = !!change.remove;
 
     // Copy or create object identifying the source (styelsheet/element) for this change.
     const source = Object.assign({}, state[sourceId], { type, href, index });
     // Copy or create collection of all rules ever changed in this source.
     const rules = Object.assign({}, source.rules);
     // Refrence or create object identifying the rule for this change.
     let rule = rules[ruleId];
     if (!rule) {
       rule = createRule({ selector, ancestors, ruleIndex }, rules);
     }
     // Copy or create collection of all CSS declarations ever added to this rule.
     const add = Object.assign({}, rule.add);
     // Copy or create collection of all CSS declarations ever removed from this rule.
     const remove = Object.assign({}, rule.remove);
 
-    if (change.remove && change.remove.property) {
-      // Track the remove operation only if the property was not previously introduced
-      // by an add operation. This ensures repeated changes of the same property
-      // register as a single remove operation of its original value.
-      if (!add[change.remove.property]) {
-        remove[change.remove.property] = change.remove.value;
-      }
+    if (hasRemove) {
+      Object.entries(change.remove).forEach(([property, value]) => {
+        // Track the remove operation only if the property was not previously introduced
+        // by an add operation. This ensures repeated changes of the same property
+        // register as a single remove operation of its original value.
+        if (!add[property]) {
+          remove[property] = value;
+        }
 
-      // Delete any previous add operation which would be canceled out by this remove.
-      if (add[change.remove.property] === change.remove.value) {
-        delete add[change.remove.property];
-      }
+        // Delete any previous add operation which would be canceled out by this remove.
+        if (add[property] === value) {
+          delete add[property];
+        }
+      });
     }
 
-    if (change.add && change.add.property) {
-      add[change.add.property] = change.add.value;
-    }
+    if (hasAdd) {
+      Object.entries(change.add).forEach(([property, value]) => {
+        add[property] = value;
 
-    const property = change.add && change.add.property ||
-                     change.remove && change.remove.property;
-
-    // Remove tracked operations if they cancel each other out.
-    if (add[property] === remove[property]) {
-      delete add[property];
-      delete remove[property];
+        // Remove previously tracked declarations if they cancel each other out.
+        if (add[property] === remove[property]) {
+          delete add[property];
+          delete remove[property];
+        }
+      });
     }
 
     // Remove information about the rule if none its declarations changed.
     if (!Object.keys(add).length && !Object.keys(remove).length) {
       removeRule(ruleId, rules);
       source.rules = { ...rules };
     } else {
       source.rules = { ...rules, [ruleId]: { ...rule, add, remove } };
--- a/devtools/server/actors/styles.js
+++ b/devtools/server/actors/styles.js
@@ -1561,35 +1561,35 @@ var StyleRuleActor = protocol.ActorClass
         // declaration is being updated. In that case, use the provided `change.name`.
         const name = change.newName ? change.newName : change.name;
         // Append the "!important" string if defined in the incoming priority flag.
         const newValue = change.priority ? `${change.value} !important` : change.value;
         // Reuse the previous value string, when the property is renamed.
         // Otherwise, use the incoming value string.
         const value = change.newName ? prevValue : newValue;
 
-        data.add = { property: name, value };
+        data.add = { [name]: value };
         // If there is a previous value, log its removal together with the previous
         // property name. Using the previous name handles the case for renaming a property
         // and is harmless when updating an existing value (the name stays the same).
-        data.remove = prevValue ? { property: prevName, value: prevValue } : null;
+        data.remove = prevValue ? { [prevName]: prevValue } : null;
 
         // When toggling a declaration from OFF to ON, if not renaming the property,
         // do not mark the previous declaration for removal, otherwise the add and
         // remove operations will cancel each other out when tracked. Tracked changes
         // have no context of "disabled", only "add" or remove, like diffs.
         if (prevDisabled && !change.newName && prevValue === newValue) {
           data.remove = null;
         }
 
         break;
 
       case "remove":
         data.add = null;
-        data.remove = { property: change.name, value: prevValue };
+        data.remove = { [change.name]: prevValue };
         break;
     }
 
     TrackChangeEmitter.trackChange(data);
   },
 
   /**
    * Calls modifySelector2() which needs to be kept around for backwards compatibility.