Bug 1303611 - cache value on attribute container to avoid redrawing unchanged attributes;r=pbro draft
authorJulian Descottes <jdescottes@mozilla.com>
Sat, 01 Oct 2016 01:36:26 +0200
changeset 420515 5796112c3f799d17068376a6a79dc12c8c96396d
parent 420514 f927dc3ad86478c7b54507fa8cf862cff4f372b4
child 532827 a2a9fbf628b56fcb6b1657f5c4351c75c345f3d6
push id31217
push userjdescottes@mozilla.com
push dateTue, 04 Oct 2016 08:02:17 +0000
reviewerspbro
bugs1303611
milestone52.0a1
Bug 1303611 - cache value on attribute container to avoid redrawing unchanged attributes;r=pbro MozReview-Commit-ID: 55cR17diVT5
devtools/client/inspector/markup/markup.js
devtools/client/inspector/markup/test/browser_markup_mutation_01.js
--- a/devtools/client/inspector/markup/markup.js
+++ b/devtools/client/inspector/markup/markup.js
@@ -3193,16 +3193,17 @@ ElementEditor.prototype = {
       attr.remove();
     }
   },
 
   _createAttribute: function (attribute, before = null) {
     // Create the template editor, which will save some variables here.
     let data = {
       attrName: attribute.name,
+      attrValue: attribute.value,
       tabindex: this.container.canFocus ? "0" : "-1",
     };
     this.template("attribute", data);
     let {attr, inner, name, val} = data;
 
     // Double quotes need to be handled specially to prevent DOMParser failing.
     // name="v"a"l"u"e" when editing -> name='v"a"l"u"e"'
     // name="v'a"l'u"e" when editing -> name="v'a&quot;l'u&quot;e"
--- a/devtools/client/inspector/markup/test/browser_markup_mutation_01.js
+++ b/devtools/client/inspector/markup/test/browser_markup_mutation_01.js
@@ -18,17 +18,19 @@ const TEST_DATA = [
   {
     desc: "Adding an attribute",
     test: function* (testActor) {
       yield testActor.setAttribute("#node1", "newattr", "newattrval");
     },
     check: function* (inspector) {
       let {editor} = yield getContainerForSelector("#node1", inspector);
       ok([...editor.attrList.querySelectorAll(".attreditor")].some(attr => {
-        return attr.textContent.trim() === "newattr=\"newattrval\"";
+        return attr.textContent.trim() === "newattr=\"newattrval\""
+          && attr.dataset.value === "newattrval"
+          && attr.dataset.attr === "newattr";
       }), "newattr attribute found");
     }
   },
   {
     desc: "Removing an attribute",
     test: function* (testActor) {
       yield testActor.removeAttribute("#node1", "newattr");
     },
@@ -42,33 +44,67 @@ const TEST_DATA = [
   {
     desc: "Re-adding an attribute",
     test: function* (testActor) {
       yield testActor.setAttribute("#node1", "newattr", "newattrval");
     },
     check: function* (inspector) {
       let {editor} = yield getContainerForSelector("#node1", inspector);
       ok([...editor.attrList.querySelectorAll(".attreditor")].some(attr => {
-        return attr.textContent.trim() === "newattr=\"newattrval\"";
+        return attr.textContent.trim() === "newattr=\"newattrval\""
+          && attr.dataset.value === "newattrval"
+          && attr.dataset.attr === "newattr";
       }), "newattr attribute found");
     }
   },
   {
     desc: "Changing an attribute",
     test: function* (testActor) {
       yield testActor.setAttribute("#node1", "newattr", "newattrchanged");
     },
     check: function* (inspector) {
       let {editor} = yield getContainerForSelector("#node1", inspector);
       ok([...editor.attrList.querySelectorAll(".attreditor")].some(attr => {
-        return attr.textContent.trim() === "newattr=\"newattrchanged\"";
+        return attr.textContent.trim() === "newattr=\"newattrchanged\""
+          && attr.dataset.value === "newattrchanged"
+          && attr.dataset.attr === "newattr";
       }), "newattr attribute found");
     }
   },
   {
+    desc: "Adding another attribute does not rerender unchanged attributes",
+    test: function* (testActor, inspector) {
+      let {editor} = yield getContainerForSelector("#node1", inspector);
+
+      // This test checks the impact on the markup-view nodes after setting attributes on
+      // content nodes.
+      info("Expect attribute-container for 'new-attr' from the previous test");
+      let attributeContainer = editor.attrList.querySelector("[data-attr=newattr]");
+      ok(attributeContainer, "attribute-container for 'newattr' found");
+
+      info("Set a flag on the attribute-container to check after the mutation");
+      attributeContainer.beforeMutationFlag = true;
+
+      info("Add the attribute 'otherattr' on the content node to trigger the mutation");
+      yield testActor.setAttribute("#node1", "otherattr", "othervalue");
+    },
+    check: function* (inspector) {
+      let {editor} = yield getContainerForSelector("#node1", inspector);
+
+      info("Check the attribute-container for the new attribute mutation was created");
+      let otherAttrContainer = editor.attrList.querySelector("[data-attr=otherattr]");
+      ok(otherAttrContainer, "attribute-container for 'otherattr' found");
+
+      info("Check the attribute-container for 'new-attr' is the same node as earlier.");
+      let newAttrContainer = editor.attrList.querySelector("[data-attr=newattr]");
+      ok(newAttrContainer, "attribute-container for 'newattr' found");
+      ok(newAttrContainer.beforeMutationFlag, "attribute-container same as earlier");
+    }
+  },
+  {
     desc: "Adding ::after element",
     numMutations: 2,
     test: function* (testActor) {
       yield testActor.eval(`
         let node1 = content.document.querySelector("#node1");
         node1.classList.add("pseudo");
       `);
     },
@@ -287,17 +323,17 @@ add_task(function* () {
       seenMutations += mutations.length;
       info("Receieved " + seenMutations +
            " mutations, expecting at least " + numMutations);
       if (seenMutations >= numMutations) {
         inspector.off("markupmutation", onmutation);
         def.resolve();
       }
     });
-    yield test(testActor);
+    yield test(testActor, inspector);
     yield def.promise;
 
     info("Expanding all markup-view nodes to make sure new nodes are imported");
     yield inspector.markup.expandAll();
 
     info("Checking the markup-view content");
     yield check(inspector);
   }