Bug 966787 - 2 - Skip inspector breadcrumb updates when the output doesn't change; r=bgrins

Whenever something changed on the selected element (pseudo, attribute), the
breadcrumbs widget used to loop over all breadcrumbs buttons and re-create the
markup for each.
Now, we cache a string version of the text displayed in a button and compare
the new value to that in the loop, to skip DOM updates.

Additionally, the breadcrumbs widget used to update itself after all markup mutations
in the DOM tree displayed in the inspector. The update method now looks at the mutations
array and early return if none of them actually impact the displayed breadcrumbs.

--HG--
extra : rebase_source : 4945c42d03e5be856c92919aa271811e66f3d188
This commit is contained in:
Patrick Brosset 2015-04-13 10:22:05 +02:00
Родитель b97af70f4e
Коммит e6e0278825
19 изменённых файлов: 304 добавлений и 64 удалений

Просмотреть файл

@ -532,7 +532,11 @@ HTMLBreadcrumbs.prototype = {
let button = this.buildButton(node);
fragment.insertBefore(button, lastButtonInserted);
lastButtonInserted = button;
this.nodeHierarchy.splice(originalLength, 0, {node, button});
this.nodeHierarchy.splice(originalLength, 0, {
node,
button,
currentPrettyPrintText: this.prettyPrintNodeAsText(node)
});
}
node = node.parentNode();
}
@ -642,22 +646,65 @@ HTMLBreadcrumbs.prototype = {
}
for (let i = this.nodeHierarchy.length - 1; i >= 0; i--) {
let crumb = this.nodeHierarchy[i];
let button = crumb.button;
let {node, button, currentPrettyPrintText} = this.nodeHierarchy[i];
while (button.hasChildNodes()) {
button.removeChild(button.firstChild);
// If the output of the node doesn't change, skip the update.
let textOutput = this.prettyPrintNodeAsText(node);
if (currentPrettyPrintText === textOutput) {
continue;
}
button.appendChild(this.prettyPrintNodeAsXUL(crumb.node));
button.setAttribute("tooltiptext", this.prettyPrintNodeAsText(crumb.node));
// Otherwise, update the whole markup for the button.
while (button.hasChildNodes()) {
button.firstChild.remove();
}
button.appendChild(this.prettyPrintNodeAsXUL(node));
button.setAttribute("tooltiptext", textOutput);
this.nodeHierarchy[i].currentPrettyPrintText = textOutput;
}
},
/**
* Given a list of mutation changes (passed by the markupmutation event),
* decide whether or not they are "interesting" to the current state of the
* breadcrumbs widget, i.e. at least one of them should cause part of the
* widget to be updated.
* @param {Array} mutations The mutations array.
* @return {Boolean}
*/
_hasInterestingMutations: function(mutations) {
if (!mutations || !mutations.length) {
return false;
}
for (let {type, added, removed, target, attributeName} of mutations) {
if (type === "childList") {
// Only interested in childList mutations if the added or removed
// nodes are currently displayed, or if it impacts the last element in
// the breadcrumbs.
return added.some(node => this.indexOf(node) > -1) ||
removed.some(node => this.indexOf(node) > -1) ||
this.indexOf(target) === this.nodeHierarchy.length - 1;
} else if (type === "attributes" && this.indexOf(target) > -1) {
// Only interested in attributes mutations if the target is
// currently displayed, and the attribute is either id or class.
return attributeName === "class" || attributeName === "id";
}
}
// Catch all return in case the mutations array was empty, or in case none
// of the changes iterated above were interesting.
return false;
},
/**
* Update the breadcrumbs display when a new node is selected.
* @param {String} reason The reason for the update, if any.
* @param {Array} mutations An array of mutations in case this was called as
* the "markupmutation" event listener.
*/
update: function(reason) {
update: function(reason, mutations) {
if (this.isDestroyed) {
return;
}
@ -666,6 +713,11 @@ HTMLBreadcrumbs.prototype = {
this.inspector.hideNodeMenu();
}
let hasInterestingMutations = this._hasInterestingMutations(mutations);
if (reason === "markupmutation" && !hasInterestingMutations) {
return;
}
let cmdDispatcher = this.chromeDoc.commandDispatcher;
this.hadFocus = (cmdDispatcher.focusedElement &&
cmdDispatcher.focusedElement.parentNode == this.container);
@ -684,7 +736,7 @@ HTMLBreadcrumbs.prototype = {
// Is the node already displayed in the breadcrumbs?
// (and there are no mutations that need re-display of the crumbs)
if (idx > -1 && reason !== "markupmutation") {
if (idx > -1 && !hasInterestingMutations) {
// Yes. We select it.
this.setCursor(idx);
} else {

Просмотреть файл

@ -29,6 +29,7 @@ support-files =
[browser_inspector_breadcrumbs.js]
[browser_inspector_breadcrumbs_highlight_hover.js]
[browser_inspector_breadcrumbs_mutations.js]
[browser_inspector_delete-selected-node-01.js]
[browser_inspector_delete-selected-node-02.js]
[browser_inspector_delete-selected-node-03.js]

Просмотреть файл

@ -0,0 +1,203 @@
/* 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 the breadcrumbs widget refreshes correctly when there are markup
// mutations (and that it doesn't refresh when those mutations don't change its
// output).
const TEST_URI = TEST_URL_ROOT + "doc_inspector_breadcrumbs.html";
// Each item in the TEST_DATA array is a test case that should contain the
// following properties:
// - desc {String} A description of this test case (will be logged).
// - setup {Function*} A generator function (can yield promises) that sets up
// the test case. Useful for selecting a node before starting the test.
// - run {Function*} A generator function (can yield promises) that runs the
// actual test case, i.e, mutates the content DOM to cause the breadcrumbs
// to refresh, or not.
// - shouldRefresh {Boolean} Once the `run` function has completed, and the test
// has detected that the page has changed, this boolean instructs the test to
// verify if the breadcrumbs has refreshed or not.
// - output {Array} A list of strings for the text that should be found in each
// button after the test has run.
const TEST_DATA = [{
desc: "Adding a child at the end of the chain should refresh and show it",
setup: function*(inspector) {
yield selectNode("#i1111", inspector);
},
run: function*({walker, selection}) {
yield walker.setInnerHTML(selection.nodeFront, "<b>test</b>");
},
shouldRefresh: true,
output: ["html", "body", "article#i1", "div#i11", "div#i111", "div#i1111", "b"]
}, {
desc: "Updating an ID to an displayed element should refresh",
setup: function*() {},
run: function*({walker}) {
let node = yield walker.querySelector(walker.rootNode, "#i1");
yield node.modifyAttributes([{
attributeName: "id",
newValue: "i1-changed"
}]);
},
shouldRefresh: true,
output: ["html", "body", "article#i1-changed", "div#i11", "div#i111", "div#i1111", "b"]
}, {
desc: "Updating an class to a displayed element should refresh",
setup: function*() {},
run: function*({walker}) {
let node = yield walker.querySelector(walker.rootNode, "body");
yield node.modifyAttributes([{
attributeName: "class",
newValue: "test-class"
}]);
},
shouldRefresh: true,
output: ["html", "body.test-class", "article#i1-changed", "div#i11", "div#i111", "div#i1111", "b"]
}, {
desc: "Updating a non id/class attribute to a displayed element should not refresh",
setup: function*() {},
run: function*({walker}) {
let node = yield walker.querySelector(walker.rootNode, "#i11");
yield node.modifyAttributes([{
attributeName: "name",
newValue: "value"
}]);
},
shouldRefresh: false,
output: ["html", "body.test-class", "article#i1-changed", "div#i11", "div#i111", "div#i1111", "b"]
}, {
desc: "Moving a child in an element that's not displayed should not refresh",
setup: function*() {},
run: function*({walker}) {
// Re-append #i1211 as a last child of #i2.
let parent = yield walker.querySelector(walker.rootNode, "#i2");
let child = yield walker.querySelector(walker.rootNode, "#i211");
yield walker.insertBefore(child, parent);
},
shouldRefresh: false,
output: ["html", "body.test-class", "article#i1-changed", "div#i11", "div#i111", "div#i1111", "b"]
}, {
desc: "Moving an undisplayed child in a displayed element should not refresh",
setup: function*() {},
run: function*({walker}) {
// Re-append #i2 in body (move it to the end).
let parent = yield walker.querySelector(walker.rootNode, "body");
let child = yield walker.querySelector(walker.rootNode, "#i2");
yield walker.insertBefore(child, parent);
},
shouldRefresh: false,
output: ["html", "body.test-class", "article#i1-changed", "div#i11", "div#i111", "div#i1111", "b"]
}, {
desc: "Updating attributes on an element that's not displayed should not refresh",
setup: function*() {},
run: function*({walker}) {
let node = yield walker.querySelector(walker.rootNode, "#i2");
yield node.modifyAttributes([{
attributeName: "id",
newValue: "i2-changed"
}, {
attributeName: "class",
newValue: "test-class"
}]);
},
shouldRefresh: false,
output: ["html", "body.test-class", "article#i1-changed", "div#i11", "div#i111", "div#i1111", "b"]
}, {
desc: "Removing the currently selected node should refresh",
setup: function*(inspector) {
yield selectNode("#i2-changed", inspector);
},
run: function*({walker, selection}) {
yield walker.removeNode(selection.nodeFront);
},
shouldRefresh: true,
output: ["html", "body.test-class", "article#i1-changed"]
}, {
desc: "Changing the class of the currently selected node should refresh",
setup: function*() {},
run: function*({selection}) {
yield selection.nodeFront.modifyAttributes([{
attributeName: "class",
newValue: "test-class-changed"
}]);
},
shouldRefresh: true,
output: ["html", "body.test-class-changed", "article#i1-changed"]
}, {
desc: "Changing the id of the currently selected node should refresh",
setup: function*() {},
run: function*({selection}) {
yield selection.nodeFront.modifyAttributes([{
attributeName: "id",
newValue: "new-id"
}]);
},
shouldRefresh: true,
output: ["html", "body#new-id.test-class-changed", "article#i1-changed"]
}];
add_task(function*() {
let {inspector} = yield openInspectorForURL(TEST_URI);
let container = inspector.panelDoc.getElementById("inspector-breadcrumbs");
let win = container.ownerDocument.defaultView;
for (let {desc, setup, run, shouldRefresh, output} of TEST_DATA) {
info("Running test case: " + desc);
info("Listen to markupmutation events from the inspector to know when a " +
"test case has completed");
let onContentMutation = inspector.once("markupmutation");
info("Running setup");
yield setup(inspector);
info("Listen to mutations on the breadcrumbs container");
let hasBreadcrumbsMutated = false;
let observer = new win.MutationObserver(mutations => {
// Only consider childList changes or tooltiptext/checked attributes
// changes. The rest may be mutations caused by the overflowing arrowbox.
for (let {type, attributeName} of mutations) {
let isChildList = type === "childList";
let isAttributes = type === "attributes" &&
(attributeName === "checked" ||
attributeName === "tooltiptext");
if (isChildList || isAttributes) {
hasBreadcrumbsMutated = true;
break;
}
}
});
observer.observe(container, {
attributes: true,
childList: true,
subtree: true
});
info("Running the test case");
yield run(inspector);
info("Wait until the page has mutated");
yield onContentMutation;
if (shouldRefresh) {
info("The breadcrumbs is expected to refresh, so wait for it");
yield inspector.once("inspector-updated");
} else {
ok(!inspector._updateProgress,
"The breadcrumbs widget is not currently updating");
}
is(shouldRefresh, hasBreadcrumbsMutated, "Has the breadcrumbs refreshed?");
observer.disconnect();
info("Check the output of the breadcrumbs widget");
is(container.childNodes.length, output.length, "Correct number of buttons");
for (let i = 0; i < container.childNodes.length; i ++) {
is(output[i], container.childNodes[i].textContent,
"Text content for button " + i + " is correct");
}
}
});

Просмотреть файл

@ -26,11 +26,9 @@ add_task(function*() {
info("wait and see if the highlighter stays visible even after the node was selected");
yield waitForTheBrieflyShowBoxModelTimeout();
let updated = inspector.once("inspector-updated");
p.textContent = "dary!!!!";
isVisible = yield isHighlighting(toolbox);
ok(isVisible, "the highlighter is still visible");
yield updated;
});
function waitForTheBrieflyShowBoxModelTimeout() {

Просмотреть файл

@ -224,16 +224,11 @@ let openLayoutView = Task.async(function*() {
});
/**
* Wait for the layoutview-updated event and for all of the inspector's panels
* to update too.
* Use this to make sure the inspector is updated and ready after a change was
* made in one of the layout-view editable fields.
* Wait for the layoutview-updated event.
* @return a promise
*/
function waitForUpdate(inspector) {
let onLayoutView = inspector.once("layoutview-updated");
let onInspector = inspector.once("inspector-updated");
return promise.all([onLayoutView, onInspector]);
return inspector.once("layoutview-updated");
}
/**

Просмотреть файл

@ -84,8 +84,6 @@ add_task(function*() {
while (inspector.markup.undo.canUndo()) {
yield undoChange(inspector);
}
yield inspector.once("inspector-updated");
});
function enterData(index, editor, inspector) {

Просмотреть файл

@ -179,10 +179,9 @@ add_task(function*() {
info("Starting test: " + desc);
info("Executing the test markup mutation");
let onUpdated = inspector.once("inspector-updated");
let onMutation = inspector.once("markupmutation");
test();
yield onUpdated.then(onMutation);
yield onMutation;
info("Expanding all markup-view nodes to make sure new nodes are imported");
yield inspector.markup.expandAll();

Просмотреть файл

@ -86,7 +86,6 @@ add_task(function*() {
info("Mutating the DOM and listening for markupmutation event");
let mutated = inspector.once("markupmutation");
let updated = inspector.once("inspector-updated");
mutate(content.document, rootNode);
yield mutated;
@ -101,9 +100,6 @@ add_task(function*() {
} else {
yield assertNodeFlashing(flashingNodeFront, inspector);
}
// Making sure the inspector has finished updating before moving on
yield updated;
}
});

Просмотреть файл

@ -28,6 +28,4 @@ add_task(function*() {
yield onMutated;
is(node.nodeValue, "New text", "Test test node's text content has changed");
yield inspector.once("inspector-updated");
});

Просмотреть файл

@ -26,8 +26,6 @@ function runAddAttributesTests(tests, nodeOrSelector, inspector) {
for (let test of tests) {
yield runAddAttributesTest(test, "div", inspector);
}
yield inspector.once("inspector-updated");
});
}

Просмотреть файл

@ -6,25 +6,20 @@
// Test cancelling the addition of a new property in the rule-view
const TEST_URI = [
"<style>",
"#testid {background-color: blue;}",
".testclass, .unmatched {background-color: green;}",
"</style>",
"<div id='testid' class='testclass'>Styled Node</div>",
"<div id='testid2'>Styled Node</div>"
].join("");
add_task(function*() {
yield addTab("data:text/html;charset=utf-8,browser_ruleview_ui.js");
yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
let {toolbox, inspector, view} = yield openRuleView();
info("Creating the test document");
let style = "" +
"#testid {" +
" background-color: blue;" +
"}" +
".testclass, .unmatched {" +
" background-color: green;" +
"}";
let styleNode = addStyle(content.document, style);
content.document.body.innerHTML = "<div id='testid' class='testclass'>Styled Node</div>" +
"<div id='testid2'>Styled Node</div>";
yield testCancelNew(inspector, view);
yield testCancelNewOnEscape(inspector, view);
yield inspector.once("inspector-updated");
});
function* testCancelNew(inspector, ruleView) {

Просмотреть файл

@ -24,7 +24,6 @@ add_task(function*() {
"<div id='testid2'>Styled Node</div>";
yield testCreateNew(inspector, view);
yield inspector.once("inspector-updated");
});
function* testCreateNew(inspector, ruleView) {
@ -60,11 +59,12 @@ function* testCreateNew(inspector, ruleView) {
let textProp = elementRuleEditor.rule.textProps[0];
is(editor, inplaceEditor(textProp.editor.valueSpan), "Should be editing the value span now.");
let onMutated = inspector.once("markupmutation");
editor.input.value = "purple";
let onBlur = once(editor.input, "blur");
EventUtils.sendKey("return", ruleView.doc.defaultView);
yield onBlur;
yield elementRuleEditor.rule._applyingModifications;
yield onMutated;
is(textProp.value, "purple", "Text prop should have been changed.");
}

Просмотреть файл

@ -23,8 +23,10 @@ add_task(function*() {
});
function* testCreateNewMultiDuplicates(inspector, ruleEditor) {
let onMutation = inspector.once("markupmutation");
yield createNewRuleViewProperty(ruleEditor,
"color:red;color:orange;color:yellow;color:green;color:blue;color:indigo;color:violet;");
yield onMutation;
is(ruleEditor.rule.textProps.length, 7, "Should have created new text properties.");
is(ruleEditor.propertyList.children.length, 8, "Should have created new property editors.");
@ -49,6 +51,4 @@ function* testCreateNewMultiDuplicates(inspector, ruleEditor) {
is(ruleEditor.rule.textProps[6].name, "color", "Should have correct property name");
is(ruleEditor.rule.textProps[6].value, "violet", "Should have correct property value");
yield inspector.once("inspector-updated");
}

Просмотреть файл

@ -23,8 +23,10 @@ add_task(function*() {
});
function* testCreateNewMultiPriority(inspector, ruleEditor) {
let onMutation = inspector.once("markupmutation");
yield createNewRuleViewProperty(ruleEditor,
"color:red;width:100px;height: 100px;");
yield onMutation;
is(ruleEditor.rule.textProps.length, 3, "Should have created new text properties.");
is(ruleEditor.propertyList.children.length, 4, "Should have created new property editors.");
@ -37,6 +39,4 @@ function* testCreateNewMultiPriority(inspector, ruleEditor) {
is(ruleEditor.rule.textProps[2].name, "height", "Should have correct property name");
is(ruleEditor.rule.textProps[2].value, "100px", "Should have correct property value");
yield inspector.once("inspector-updated");
}

Просмотреть файл

@ -23,8 +23,10 @@ add_task(function*() {
});
function* testCreateNewMultiUnfinished(inspector, ruleEditor, view) {
let onMutation = inspector.once("markupmutation");
yield createNewRuleViewProperty(ruleEditor,
"color:blue;background : orange ; text-align:center; border-color: ");
yield onMutation;
is(ruleEditor.rule.textProps.length, 4, "Should have created new text properties.");
is(ruleEditor.propertyList.children.length, 4, "Should have created property editors.");
@ -48,6 +50,4 @@ function* testCreateNewMultiUnfinished(inspector, ruleEditor, view) {
is(ruleEditor.rule.textProps[3].name, "border-color", "Should have correct property name");
is(ruleEditor.rule.textProps[3].value, "red", "Should have correct property value");
yield inspector.once("inspector-updated");
}

Просмотреть файл

@ -23,15 +23,19 @@ add_task(function*() {
});
function* testCreateNewMultiPartialUnfinished(inspector, ruleEditor, view) {
let onMutation = inspector.once("markupmutation");
yield createNewRuleViewProperty(ruleEditor, "width: 100px; heig");
yield onMutation;
is(ruleEditor.rule.textProps.length, 2, "Should have created a new text property.");
is(ruleEditor.propertyList.children.length, 2, "Should have created a property editor.");
// Value is focused, lets add multiple rules here and make sure they get added
onMutation = inspector.once("markupmutation");
let valueEditor = ruleEditor.propertyList.children[1].querySelector("input");
valueEditor.value = "10px;background:orangered;color: black;";
EventUtils.synthesizeKey("VK_RETURN", {}, view.doc.defaultView);
yield onMutation;
is(ruleEditor.rule.textProps.length, 4, "Should have added the changed value.");
is(ruleEditor.propertyList.children.length, 5, "Should have added the changed value editor.");
@ -47,6 +51,4 @@ function* testCreateNewMultiPartialUnfinished(inspector, ruleEditor, view) {
is(ruleEditor.rule.textProps[3].name, "color", "Should have correct property name");
is(ruleEditor.rule.textProps[3].value, "black", "Should have correct property value");
yield inspector.once("inspector-updated");
}

Просмотреть файл

@ -23,8 +23,10 @@ add_task(function*() {
});
function* testCreateNewMulti(inspector, ruleEditor) {
let onMutation = inspector.once("markupmutation");
yield createNewRuleViewProperty(ruleEditor,
"color:blue;background : orange ; text-align:center; border-color: green;");
yield onMutation;
is(ruleEditor.rule.textProps.length, 4, "Should have created a new text property.");
is(ruleEditor.propertyList.children.length, 5, "Should have created a new property editor.");
@ -40,6 +42,4 @@ function* testCreateNewMulti(inspector, ruleEditor) {
is(ruleEditor.rule.textProps[3].name, "border-color", "Should have correct property name");
is(ruleEditor.rule.textProps[3].value, "green", "Should have correct property value");
yield inspector.once("inspector-updated");
}

Просмотреть файл

@ -29,9 +29,11 @@ function* testMultiValues(inspector, ruleEditor, view) {
is(ruleEditor.propertyList.children.length, 1, "Should have created a property editor.");
// Value is focused, lets add multiple rules here and make sure they get added
let onMutation = inspector.once("markupmutation");
let valueEditor = ruleEditor.propertyList.children[0].querySelector("input");
valueEditor.value = "height: 10px;color:blue"
EventUtils.synthesizeKey("VK_RETURN", {}, view.doc.defaultView);
yield onMutation;
is(ruleEditor.rule.textProps.length, 2, "Should have added the changed value.");
is(ruleEditor.propertyList.children.length, 3, "Should have added the changed value editor.");
@ -44,6 +46,4 @@ function* testMultiValues(inspector, ruleEditor, view) {
is(ruleEditor.rule.textProps[1].name, "color", "Should have correct property name");
is(ruleEditor.rule.textProps[1].value, "blue", "Should have correct property value");
yield inspector.once("inspector-updated");
}

Просмотреть файл

@ -25,6 +25,12 @@ function* createTestContent(inspector, style) {
return styleNode;
}
function* removeTestContent(inspector, node) {
let onMutated = inspector.once("markupmutation");
node.remove();
yield onMutated;
}
function* simpleOverride(inspector, view) {
let styleNode = yield createTestContent(inspector, '' +
'#testid {' +
@ -57,7 +63,7 @@ function* simpleOverride(inspector, view) {
ok(idProp.overridden, "ID property should be overridden");
ok(classProp.overridden, "Class property should be overridden");
styleNode.remove();
yield removeTestContent(inspector, styleNode);
}
function* partialOverride(inspector, view) {
@ -86,7 +92,7 @@ function* partialOverride(inspector, view) {
}
}
styleNode.remove();
yield removeTestContent(inspector, styleNode);
}
function* importantOverride(inspector, view) {
@ -110,7 +116,7 @@ function* importantOverride(inspector, view) {
let classProp = classRule.textProps[0];
ok(!classProp.overridden, "Important rule should not be overridden.");
styleNode.remove();
yield removeTestContent(inspector, styleNode);
let elementRule = elementStyle.rules[0];
let elementProp = elementRule.createProperty("background-color", "purple", "important");
@ -141,6 +147,5 @@ function* disableOverride(inspector, view) {
let classProp = classRule.textProps[0];
ok(!classProp.overridden, "Class prop should not be overridden after id prop was disabled.");
styleNode.remove();
yield inspector.once("inspector-updated");
yield removeTestContent(inspector, styleNode);
}