Bug 1447736 - Refresh rule/computed-view even when parents and siblings change; r=jdescottes

MozReview-Commit-ID: 9RZyWwnpgUj

--HG--
extra : rebase_source : 9477160337327fa4059ea88871fc76519379c803
This commit is contained in:
Patrick Brosset 2018-03-28 17:20:22 +02:00
Родитель 441318a537
Коммит 1439d8acd8
10 изменённых файлов: 367 добавлений и 185 удалений

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

@ -1414,16 +1414,13 @@ function ComputedViewTool(inspector, window) {
this.onSelected = this.onSelected.bind(this);
this.refresh = this.refresh.bind(this);
this.onPanelSelected = this.onPanelSelected.bind(this);
this.onMutations = this.onMutations.bind(this);
this.onResized = this.onResized.bind(this);
this.inspector.selection.on("detached-front", this.onDetachedFront);
this.inspector.selection.on("new-node-front", this.onSelected);
this.inspector.selection.on("pseudoclass", this.refresh);
this.inspector.sidebar.on("computedview-selected", this.onPanelSelected);
this.inspector.pageStyle.on("stylesheet-updated", this.refresh);
this.inspector.walker.on("mutations", this.onMutations);
this.inspector.walker.on("resize", this.onResized);
this.inspector.styleChangeTracker.on("style-changed", this.refresh);
this.computedView.selectElement(null);
@ -1487,31 +1484,8 @@ ComputedViewTool.prototype = {
}
},
/**
* When markup mutations occur, if an attribute of the selected node changes,
* we need to refresh the view as that might change the node's styles.
*/
onMutations: function(mutations) {
for (let {type, target} of mutations) {
if (target === this.inspector.selection.nodeFront &&
type === "attributes") {
this.refresh();
break;
}
}
},
/**
* When the window gets resized, this may cause media-queries to match, and
* therefore, different styles may apply.
*/
onResized: function() {
this.refresh();
},
destroy: function() {
this.inspector.walker.off("mutations", this.onMutations);
this.inspector.walker.off("resize", this.onResized);
this.inspector.styleChangeTracker.off("style-changed", this.refresh);
this.inspector.sidebar.off("computedview-selected", this.refresh);
this.inspector.selection.off("pseudoclass", this.refresh);
this.inspector.selection.off("new-node-front", this.onSelected);

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

@ -17,6 +17,7 @@ const Telemetry = require("devtools/client/shared/telemetry");
const HighlightersOverlay = require("devtools/client/inspector/shared/highlighters-overlay");
const ReflowTracker = require("devtools/client/inspector/shared/reflow-tracker");
const Store = require("devtools/client/inspector/store");
const InspectorStyleChangeTracker = require("devtools/client/inspector/shared/style-change-tracker");
// Use privileged promise in panel documents to prevent having them to freeze
// during toolbox destruction. See bug 1402779.
@ -104,6 +105,7 @@ function Inspector(toolbox) {
this.highlighters = new HighlightersOverlay(this);
this.prefsObserver = new PrefObserver("devtools.");
this.reflowTracker = new ReflowTracker(this._target);
this.styleChangeTracker = new InspectorStyleChangeTracker(this);
this.store = Store();
this.telemetry = new Telemetry();
@ -1314,6 +1316,7 @@ Inspector.prototype = {
this.highlighters.destroy();
this.prefsObserver.destroy();
this.reflowTracker.destroy();
this.styleChangeTracker.destroy();
this.search.destroy();
this._toolbox = null;

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

@ -1748,9 +1748,8 @@ function RuleViewTool(inspector, window) {
this.clearUserProperties = this.clearUserProperties.bind(this);
this.refresh = this.refresh.bind(this);
this.onMutations = this.onMutations.bind(this);
this.onDetachedFront = this.onDetachedFront.bind(this);
this.onPanelSelected = this.onPanelSelected.bind(this);
this.onResized = this.onResized.bind(this);
this.onSelected = this.onSelected.bind(this);
this.onViewRefreshed = this.onViewRefreshed.bind(this);
@ -1762,8 +1761,7 @@ function RuleViewTool(inspector, window) {
this.inspector.ruleViewSideBar.on("ruleview-selected", this.onPanelSelected);
this.inspector.sidebar.on("ruleview-selected", this.onPanelSelected);
this.inspector.pageStyle.on("stylesheet-updated", this.refresh);
this.inspector.walker.on("mutations", this.onMutations);
this.inspector.walker.on("resize", this.onResized);
this.inspector.styleChangeTracker.on("style-changed", this.refresh);
this.onSelected();
}
@ -1836,31 +1834,8 @@ RuleViewTool.prototype = {
this.inspector.emit("rule-view-refreshed");
},
/**
* When markup mutations occur, if an attribute of the selected node changes,
* we need to refresh the view as that might change the node's styles.
*/
onMutations: function(mutations) {
for (let {type, target} of mutations) {
if (target === this.inspector.selection.nodeFront &&
type === "attributes") {
this.refresh();
break;
}
}
},
/**
* When the window gets resized, this may cause media-queries to match, and
* therefore, different styles may apply.
*/
onResized: function() {
this.refresh();
},
destroy: function() {
this.inspector.walker.off("mutations", this.onMutations);
this.inspector.walker.off("resize", this.onResized);
this.inspector.styleChangeTracker.off("style-changed", this.refresh);
this.inspector.selection.off("detached-front", this.onDetachedFront);
this.inspector.selection.off("pseudoclass", this.refresh);
this.inspector.selection.off("new-node-front", this.onSelected);

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

@ -13,141 +13,141 @@ const TEST_URI = `
</div>
`;
// The series of test cases to run. Each case is an object with the following properties:
// - {String} desc The test case description
// - {Function} setup The setup function to execute for this case
// - {Array} properties The properties to expect as a result. Each of them is an object:
// - {String} name The expected property name
// - {String} value The expected property value
// - {Boolean} overridden The expected property overridden state
// - {Boolean} enabled The expected property enabled state
const TEST_DATA = [{
desc: "Adding a second margin-top value in the element selector",
setup: async function({ view }) {
await addProperty(view, 0, "margin-top", "5px");
},
properties: [
{ name: "margin-top", value: "1px", overridden: true, enabled: true },
{ name: "padding-top", value: "5px", overridden: false, enabled: true },
{ name: "margin-top", value: "5px", overridden: false, enabled: true }
]
}, {
desc: "Setting the element style to its original value",
setup: async function({ inspector, testActor }) {
await changeElementStyle("#testid", "margin-top: 1px; padding-top: 5px", inspector,
testActor);
},
properties: [
{ name: "margin-top", value: "1px", overridden: false, enabled: true },
{ name: "padding-top", value: "5px", overridden: false, enabled: true },
{ name: "margin-top", value: "5px", overridden: false, enabled: false }
]
}, {
desc: "Set the margin-top back to 5px, the previous property should be re-enabled",
setup: async function({ inspector, testActor }) {
await changeElementStyle("#testid", "margin-top: 5px; padding-top: 5px;", inspector,
testActor);
},
properties: [
{ name: "margin-top", value: "1px", overridden: false, enabled: false },
{ name: "padding-top", value: "5px", overridden: false, enabled: true },
{ name: "margin-top", value: "5px", overridden: false, enabled: true }
]
}, {
desc: "Set the margin property to a value that doesn't exist in the editor, which " +
"should reuse the currently re-enabled property (the second one)",
setup: async function({ inspector, testActor }) {
await changeElementStyle("#testid", "margin-top: 15px; padding-top: 5px;", inspector,
testActor);
},
properties: [
{ name: "margin-top", value: "1px", overridden: false, enabled: false },
{ name: "padding-top", value: "5px", overridden: false, enabled: true },
{ name: "margin-top", value: "15px", overridden: false, enabled: true }
]
}, {
desc: "Remove the padding-top attribute. Should disable the padding property but not " +
"remove it",
setup: async function({ inspector, testActor }) {
await changeElementStyle("#testid", "margin-top: 5px;", inspector, testActor);
},
properties: [
{ name: "margin-top", value: "1px", overridden: false, enabled: false },
{ name: "padding-top", value: "5px", overridden: false, enabled: false },
{ name: "margin-top", value: "5px", overridden: false, enabled: true }
]
}, {
desc: "Put the padding-top attribute back in, should re-enable the padding property",
setup: async function({ inspector, testActor }) {
await changeElementStyle("#testid", "margin-top: 5px; padding-top: 25px", inspector,
testActor);
},
properties: [
{ name: "margin-top", value: "1px", overridden: false, enabled: false },
{ name: "padding-top", value: "25px", overridden: false, enabled: true },
{ name: "margin-top", value: "5px", overridden: false, enabled: true }
]
}, {
desc: "Add an entirely new property",
setup: async function({ inspector, testActor }) {
await changeElementStyle("#testid",
"margin-top: 5px; padding-top: 25px; padding-left: 20px;",
inspector, testActor);
},
properties: [
{ name: "margin-top", value: "1px", overridden: false, enabled: false },
{ name: "padding-top", value: "25px", overridden: false, enabled: true },
{ name: "margin-top", value: "5px", overridden: false, enabled: true },
{ name: "padding-left", value: "20px", overridden: false, enabled: true }
]
}, {
desc: "Add an entirely new property again",
setup: async function({ inspector, testActor }) {
await changeElementStyle("#testid", "color: red", inspector, testActor);
},
properties: [
{ name: "margin-top", value: "1px", overridden: false, enabled: false },
{ name: "padding-top", value: "25px", overridden: false, enabled: false },
{ name: "margin-top", value: "5px", overridden: false, enabled: false },
{ name: "padding-left", value: "20px", overridden: false, enabled: false },
{ name: "color", value: "red", overridden: false, enabled: true }
]
}];
add_task(async function() {
await addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
let {inspector, view, testActor} = await openRuleView();
let { inspector, view, testActor } = await openRuleView();
await selectNode("#testid", inspector);
await testPropertyChanges(inspector, view);
await testPropertyChange0(inspector, view, "#testid", testActor);
await testPropertyChange1(inspector, view, "#testid", testActor);
await testPropertyChange2(inspector, view, "#testid", testActor);
await testPropertyChange3(inspector, view, "#testid", testActor);
await testPropertyChange4(inspector, view, "#testid", testActor);
await testPropertyChange5(inspector, view, "#testid", testActor);
await testPropertyChange6(inspector, view, "#testid", testActor);
for (let { desc, setup, properties } of TEST_DATA) {
info(desc);
await setup({ inspector, view, testActor });
let rule = view._elementStyle.rules[0];
is(rule.editor.element.querySelectorAll(".ruleview-property").length,
properties.length, "The correct number of properties was found");
properties.forEach(({ name, value, overridden, enabled }, index) => {
validateTextProp(rule.textProps[index], overridden, enabled, name, value);
});
}
});
async function testPropertyChanges(inspector, ruleView) {
info("Adding a second margin-top value in the element selector");
let ruleEditor = ruleView._elementStyle.rules[0].editor;
let onRefreshed = inspector.once("rule-view-refreshed");
ruleEditor.addProperty("margin-top", "5px", "", true);
await onRefreshed;
let rule = ruleView._elementStyle.rules[0];
validateTextProp(rule.textProps[0], false, "margin-top", "1px",
"Original margin property active");
}
async function testPropertyChange0(inspector, ruleView, selector, testActor) {
await changeElementStyle(selector, "margin-top: 1px; padding-top: 5px",
inspector, testActor);
let rule = ruleView._elementStyle.rules[0];
is(rule.editor.element.querySelectorAll(".ruleview-property").length, 3,
"Correct number of properties");
validateTextProp(rule.textProps[0], true, "margin-top", "1px",
"First margin property re-enabled");
validateTextProp(rule.textProps[2], false, "margin-top", "5px",
"Second margin property disabled");
}
async function testPropertyChange1(inspector, ruleView, selector, testActor) {
info("Now set it back to 5px, the 5px value should be re-enabled.");
await changeElementStyle(selector, "margin-top: 5px; padding-top: 5px;",
inspector, testActor);
let rule = ruleView._elementStyle.rules[0];
is(rule.editor.element.querySelectorAll(".ruleview-property").length, 3,
"Correct number of properties");
validateTextProp(rule.textProps[0], false, "margin-top", "1px",
"First margin property re-enabled");
validateTextProp(rule.textProps[2], true, "margin-top", "5px",
"Second margin property disabled");
}
async function testPropertyChange2(inspector, ruleView, selector, testActor) {
info("Set the margin property to a value that doesn't exist in the editor.");
info("Should reuse the currently-enabled element (the second one.)");
await changeElementStyle(selector, "margin-top: 15px; padding-top: 5px;",
inspector, testActor);
let rule = ruleView._elementStyle.rules[0];
is(rule.editor.element.querySelectorAll(".ruleview-property").length, 3,
"Correct number of properties");
validateTextProp(rule.textProps[0], false, "margin-top", "1px",
"First margin property re-enabled");
validateTextProp(rule.textProps[2], true, "margin-top", "15px",
"Second margin property disabled");
}
async function testPropertyChange3(inspector, ruleView, selector, testActor) {
info("Remove the padding-top attribute. Should disable the padding " +
"property but not remove it.");
await changeElementStyle(selector, "margin-top: 5px;", inspector, testActor);
let rule = ruleView._elementStyle.rules[0];
is(rule.editor.element.querySelectorAll(".ruleview-property").length, 3,
"Correct number of properties");
validateTextProp(rule.textProps[1], false, "padding-top", "5px",
"Padding property disabled");
}
async function testPropertyChange4(inspector, ruleView, selector, testActor) {
info("Put the padding-top attribute back in, should re-enable the " +
"padding property.");
await changeElementStyle(selector, "margin-top: 5px; padding-top: 25px",
inspector, testActor);
let rule = ruleView._elementStyle.rules[0];
is(rule.editor.element.querySelectorAll(".ruleview-property").length, 3,
"Correct number of properties");
validateTextProp(rule.textProps[1], true, "padding-top", "25px",
"Padding property enabled");
}
async function testPropertyChange5(inspector, ruleView, selector, testActor) {
info("Add an entirely new property");
await changeElementStyle(selector,
"margin-top: 5px; padding-top: 25px; padding-left: 20px;",
inspector, testActor);
let rule = ruleView._elementStyle.rules[0];
is(rule.editor.element.querySelectorAll(".ruleview-property").length, 4,
"Added a property");
validateTextProp(rule.textProps[3], true, "padding-left", "20px",
"Padding property enabled");
}
async function testPropertyChange6(inspector, ruleView, selector, testActor) {
info("Add an entirely new property again");
await changeElementStyle(selector, "background: red " +
"url(\"chrome://branding/content/about-logo.png\") repeat scroll 0% 0%",
inspector, testActor);
let rule = ruleView._elementStyle.rules[0];
is(rule.editor.element.querySelectorAll(".ruleview-property").length, 5,
"Added a property");
validateTextProp(rule.textProps[4], true, "background",
"red url(\"chrome://branding/content/about-logo.png\") repeat scroll 0% 0%",
"shortcut property correctly set");
}
async function changeElementStyle(selector, style, inspector, testActor) {
info(`Setting ${selector}'s element style to ${style}`);
let onRefreshed = inspector.once("rule-view-refreshed");
await testActor.setAttribute(selector, "style", style);
await onRefreshed;
}
function validateTextProp(prop, enabled, name, value, desc) {
is(prop.enabled, enabled, desc + ": enabled.");
is(prop.name, name, desc + ": name.");
is(prop.value, value, desc + ": value.");
function validateTextProp(prop, overridden, enabled, name, value) {
is(prop.name, name, `${name} property name is correct`);
is(prop.editor.nameSpan.textContent, name, `${name} property name is correct in UI`);
is(prop.editor.enable.hasAttribute("checked"), enabled,
desc + ": enabled checkbox.");
is(prop.editor.nameSpan.textContent, name, desc + ": name span.");
is(prop.editor.valueSpan.textContent,
value, desc + ": value span.");
is(prop.value, value, `${name} property value is correct`);
is(prop.editor.valueSpan.textContent, value, `${name} property value is correct in UI`);
is(prop.enabled, enabled, `${name} property enabled state correct`);
is(prop.overridden, overridden, `${name} property overridden state correct`);
}

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

@ -247,9 +247,7 @@ var openCubicBezierAndChangeCoords = async function(view, ruleIndex,
* @param {CssRuleView} view
* The instance of the rule-view panel
* @param {Number} ruleIndex
* The index of the rule to use. Note that if ruleIndex is 0, you might
* want to also listen to markupmutation events in your test since
* that's going to change the style attribute of the selected node.
* The index of the rule to use.
* @param {String} name
* The name for the new property
* @param {String} value
@ -265,14 +263,34 @@ var openCubicBezierAndChangeCoords = async function(view, ruleIndex,
* @return {TextProperty} The instance of the TextProperty that was added
*/
var addProperty = async function(view, ruleIndex, name, value,
commitValueWith = "VK_RETURN",
blurNewProperty = true) {
commitValueWith = "VK_RETURN",
blurNewProperty = true) {
info("Adding new property " + name + ":" + value + " to rule " + ruleIndex);
let ruleEditor = getRuleViewRuleEditor(view, ruleIndex);
let editor = await focusNewRuleViewProperty(ruleEditor);
let numOfProps = ruleEditor.rule.textProps.length;
let onMutations = new Promise(r => {
// If we're adding the property to a non-element style rule, we don't need to wait
// for mutations.
if (ruleIndex !== 0) {
r();
}
// Otherwise, adding the property to the element style rule causes 2 mutations to the
// style attribute on the element: first when the name is added with an empty value,
// and then when the value is added.
let receivedMutations = 0;
view.inspector.walker.on("mutations", function onWalkerMutations(mutations) {
receivedMutations += mutations.length;
if (receivedMutations >= 2) {
view.inspector.walker.off("mutations", onWalkerMutations);
r();
}
});
});
info("Adding name " + name);
editor.input.value = name;
let onNameAdded = view.once("ruleview-changed");
@ -301,6 +319,9 @@ var addProperty = async function(view, ruleIndex, name, value,
EventUtils.synthesizeKey(commitValueWith, {}, view.styleWindow);
await onValueAdded;
info("Waiting for DOM mutations in case the property was added to the element style");
await onMutations;
if (blurNewProperty) {
view.styleDocument.activeElement.blur();
}

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

@ -9,6 +9,7 @@ DevToolsModules(
'highlighters-overlay.js',
'node-types.js',
'reflow-tracker.js',
'style-change-tracker.js',
'style-inspector-menu.js',
'tooltips-overlay.js',
'utils.js'

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

@ -0,0 +1,98 @@
/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
/* 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/. */
"use strict";
const EventEmitter = require("devtools/shared/event-emitter");
/**
* The InspectorStyleChangeTracker simply emits an event when it detects any changes in
* the page that may cause the current inspector selection to have different style applied
* to it.
* It currently tracks:
* - markup mutations, because they may cause different CSS rules to apply to the current
* node.
* - window resize, because they may cause media query changes and therefore also
* different CSS rules to apply to the current node.
*/
class InspectorStyleChangeTracker {
constructor(inspector) {
this.walker = inspector.walker;
this.selection = inspector.selection;
this.onMutations = this.onMutations.bind(this);
this.onResized = this.onResized.bind(this);
this.walker.on("mutations", this.onMutations);
this.walker.on("resize", this.onResized);
EventEmitter.decorate(this);
}
destroy() {
this.walker.off("mutations", this.onMutations);
this.walker.off("resize", this.onResized);
this.walker = this.selection = null;
}
/**
* When markup mutations occur, if an attribute of the selected node, one of its
* ancestors or siblings changes, we need to consider this as potentially causing a
* style change for the current node.
*/
onMutations(mutations) {
const canMutationImpactCurrentStyles = ({ type, target }) => {
// Only attributes mutations are interesting here.
if (type !== "attributes") {
return false;
}
// Is the mutation on the current selected node?
let currentNode = this.selection.nodeFront;
if (target === currentNode) {
return true;
}
// Is the mutation on one of the current selected node's siblings?
// We can't know the order of nodes on the client-side without calling
// walker.children, so don't attempt to check the previous or next element siblings.
// It's good enough to know that one sibling changed.
let parent = currentNode.parentNode();
let siblings = parent.treeChildren();
if (siblings.includes(target)) {
return true;
}
// Is the mutation on one of the current selected node's parents?
while (parent) {
if (target === parent) {
return true;
}
parent = parent.parentNode();
}
return false;
};
for (let mutation of mutations) {
if (canMutationImpactCurrentStyles(mutation)) {
this.emit("style-changed");
break;
}
}
}
/**
* When the window gets resized, this may cause media-queries to match, and we therefore
* need to consider this as a style change for the current node.
*/
onResized() {
this.emit("style-changed");
}
}
module.exports = InspectorStyleChangeTracker;

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

@ -3,6 +3,7 @@ tags = devtools
subsuite = devtools
support-files =
doc_author-sheet.html
doc_content_style_changes.html
doc_content_stylesheet.html
doc_content_stylesheet.xul
doc_content_stylesheet_imported.css
@ -30,6 +31,7 @@ skip-if = (os == 'linux' && bits == 32 && debug) # bug 1328915, disable linux32
skip-if = e10s && debug # Bug 1250058 (docshell leak when opening 2 toolboxes)
[browser_styleinspector_output-parser.js]
[browser_styleinspector_refresh_when_active.js]
[browser_styleinspector_refresh_when_style_changes.js]
[browser_styleinspector_tooltip-background-image.js]
[browser_styleinspector_tooltip-closes-on-new-selection.js]
skip-if = e10s # Bug 1111546 (e10s)

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

@ -0,0 +1,80 @@
/* 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 rule and computed views refresh when style changes that impact the
// current selection occur.
// This test does not need to worry about the correctness of the styles and rules
// displayed in these views (other tests do this) but only cares that they do catch the
// change.
const TEST_URI = TEST_URL_ROOT + "doc_content_style_changes.html";
const TEST_DATA = [{
target: "#test",
className: "green-class",
force: true
}, {
target: "#test",
className: "green-class",
force: false
}, {
target: "#parent",
className: "purple-class",
force: true
}, {
target: "#parent",
className: "purple-class",
force: false
}, {
target: "#sibling",
className: "blue-class",
force: true
}, {
target: "#sibling",
className: "blue-class",
force: false
}];
add_task(async function() {
let tab = await addTab(TEST_URI);
let { inspector } = await openRuleView();
await selectNode("#test", inspector);
info("Run the test on the rule-view");
await runViewTest(inspector, tab, "rule");
info("Switch to the computed view");
let onComputedViewReady = inspector.once("computed-view-refreshed");
selectComputedView(inspector);
await onComputedViewReady;
info("Run the test again on the computed view");
await runViewTest(inspector, tab, "computed");
});
async function runViewTest(inspector, tab, viewName) {
for (let { target, className, force } of TEST_DATA) {
info((force ? "Adding" : "Removing") +
` class ${className} on ${target} and expecting a ${viewName}-view refresh`);
await toggleClassAndWaitForViewChange(
{ target, className, force }, inspector, tab, `${viewName}-view-refreshed`);
}
}
async function toggleClassAndWaitForViewChange(whatToMutate, inspector, tab, eventName) {
let onRefreshed = inspector.once(eventName);
await ContentTask.spawn(tab.linkedBrowser, whatToMutate,
function({ target, className, force }) {
content.document.querySelector(target).classList.toggle(className, force);
}
);
await onRefreshed;
ok(true, "The view was refreshed after the class was changed");
}

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

@ -0,0 +1,28 @@
<!DOCTYPE html>
<meta charset="utf-8">
<style>
#test {
color: red;
}
/* Adding/removing the green-class on #test should refresh the rule-view when #test is
selected */
#test.green-class {
color: green;
}
/* Adding/removing the purple-class on #parent should refresh the rule-view when #test is
selected */
#parent.purple-class #test {
color: purple;
}
/* Adding/removing the blue-class on #sibling should refresh the rule-view when #test is
selected*/
#sibling.blue-class + #test {
color: blue;
}
</style>
<div id="parent">
<div>
<div id="sibling"></div>
<div id="test">test</div>
</div>
</div>