Bug 1509620 - Computed style inspector CSS cascade calculation is wrong r=ladybenko

### Try

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f1bad5e5282812225da95c0ea9e2ef173640b5da

### Summary

!!Comparing numerous complex websites such as github, facebook, cnn etc. the cascade now matchers that of Chrome so we are in a much better place.!!

According to https://www.w3.org/TR/css-cascade-3/#cascading (which platform follows now) and https://www.w3.org/TR/css-cascade-4/#cascading we should now be doing this (descending order):

  - Transition declarations
  - User-Agent & !important
  - User & !important
  - Author & !important
  - CSS Animations, @keyframes
  - Author, normal weight
  - User, normal weight
  - User-Agent, normal weight
  - specificity
  - Sheet Index
  - Rule Line
  - Rule Column

We are only dealing with CSS selectors here so we can safely drop Transition declarations and CSS Animations because their presence here is irrelevant when it comes to the CSS cascade information we display in the computed view.

This leaves us with:

  - User-Agent & !important
  - User & !important
  - Author & !important
  - Author, normal weight
  - User, normal weight
  - User-Agent, normal weight
  - specificity
  - Sheet Index
  - Rule Line
  - Rule Column

### Changes

- References to content stylesheets have been changed to author stylesheet to closely match the technical terms author, user and agent stylesheets.
- Simplified and modernized a bunch of for loops to make the code easier to understand.
- Previous to these changes all matching parent rules were classed as equal e.g. color on the body tag was equal to color on a node's immediate container. We now use the `distance` variable to tell how close a rule is to the current node. This is the highest qualifier in our cascade calculation.
- The `_agentSheet`, `_authorSheet` and `_userSheet` properties are now used to obtain a sheets origin.
- `elementStyle` was renamed to `inlineStyle` in order to correctly identify the rule's origin.
- We used to sort the matchedSelectors to move rules with `STATUS.MATCHED` above `STATUS.PARENT_MATCH` but this is unnecessary now that we have the `distance` property so we no longer do this.
- The `compareTo()` method has been updated to match https://www.w3.org/TR/css-cascade-3/#cascading (which platform follows now) and https://www.w3.org/TR/css-cascade-4/#cascading. It has also been simplified and made far less prone to error.

Differential Revision: https://phabricator.services.mozilla.com/D23711

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Michael Ratcliffe 2019-03-25 16:32:19 +00:00
Родитель 59aaa3ef97
Коммит 639bed7bb2
5 изменённых файлов: 266 добавлений и 118 удалений

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

@ -15,7 +15,7 @@ const {
} = require("devtools/shared/layout/utils");
const defer = require("devtools/shared/defer");
const {
isContentStylesheet,
isAuthorStylesheet,
getCSSStyleRules,
} = require("devtools/shared/inspector/css-logic");
const InspectorUtils = require("InspectorUtils");
@ -773,7 +773,7 @@ var TestActor = exports.TestActor = protocol.ActorClassWithSpec(testSpec, {
const sheet = domRules[i].parentStyleSheet;
sheets.push({
href: sheet.href,
isContentSheet: isContentStylesheet(sheet),
isContentSheet: isAuthorStylesheet(sheet),
});
}

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

@ -35,13 +35,20 @@ const {
getBindingElementAndPseudo,
getCSSStyleRules,
l10n,
isContentStylesheet,
isAgentStylesheet,
isAuthorStylesheet,
isUserStylesheet,
shortSource,
FILTER,
STATUS,
} = require("devtools/shared/inspector/css-logic");
const InspectorUtils = require("InspectorUtils");
const COMPAREMODE = {
"BOOLEAN": "bool",
"INTEGER": "int",
};
/**
* @param {function} isInherited A function that determines if the CSS property
* is inherited.
@ -170,8 +177,7 @@ CssLogic.prototype = {
// Update the CssSheet objects.
this.forEachSheet(function(sheet) {
sheet._sheetAllowed = -1;
if (sheet.contentSheet && sheet.sheetAllowed) {
if (sheet.authorSheet && sheet.sheetAllowed) {
ruleCount += sheet.ruleCount;
}
}, this);
@ -281,7 +287,7 @@ CssLogic.prototype = {
const sheets = [];
this.forEachSheet(function(sheet) {
if (sheet.contentSheet) {
if (sheet.authorSheet) {
sheets.push(sheet);
}
}, this);
@ -324,10 +330,7 @@ CssLogic.prototype = {
let sheetFound = false;
if (cacheId in this._sheets) {
for (let i = 0, numSheets = this._sheets[cacheId].length;
i < numSheets;
i++) {
sheet = this._sheets[cacheId][i];
for (sheet of this._sheets[cacheId]) {
if (sheet.domSheet === domSheet) {
if (index != -1) {
sheet.index = index;
@ -344,7 +347,7 @@ CssLogic.prototype = {
}
sheet = new CssSheet(this, domSheet, index);
if (sheet.sheetAllowed && sheet.contentSheet) {
if (sheet.sheetAllowed && sheet.authorSheet) {
this._ruleCount += sheet.ruleCount;
}
@ -437,19 +440,18 @@ CssLogic.prototype = {
this._matchedSelectors = [];
this._passId++;
for (let i = 0; i < this._matchedRules.length; i++) {
const rule = this._matchedRules[i][0];
const status = this._matchedRules[i][1];
for (const matchedRule of this._matchedRules) {
const [rule, status, distance] = matchedRule;
rule.selectors.forEach(function(selector) {
if (selector._matchId !== this._matchId &&
(selector.elementStyle ||
(selector.inlineStyle ||
this.selectorMatchesElement(rule.domRule,
selector.selectorIndex))) {
selector._matchId = this._matchId;
this._matchedSelectors.push([ selector, status ]);
this._matchedSelectors.push([ selector, status, distance ]);
if (callback) {
callback.call(scope, selector, status);
callback.call(scope, selector, status, distance);
}
}
}, this);
@ -532,6 +534,13 @@ CssLogic.prototype = {
const filter = this.sourceFilter;
let sheetIndex = 0;
// distance is used to tell us how close an ancestor is to an element e.g.
// 0: The rule is directly applied to the current element.
// -1: The rule is inherited from the current element's first parent.
// -2: The rule is inherited from the current element's second parent.
// etc.
let distance = 0;
this._matchId++;
this._passId++;
this._matchedRules = [];
@ -552,9 +561,7 @@ CssLogic.prototype = {
}
// getCSSStyleRules can return null with a shadow DOM element.
const numDomRules = domRules ? domRules.length : 0;
for (let i = 0; i < numDomRules; i++) {
const domRule = domRules[i];
for (const domRule of domRules || []) {
if (domRule.type !== CSSRule.STYLE_RULE) {
continue;
}
@ -565,7 +572,7 @@ CssLogic.prototype = {
sheet._passId = this._passId;
}
if (filter === FILTER.USER && !sheet.contentSheet) {
if (filter === FILTER.USER && !sheet.authorSheet) {
continue;
}
@ -576,7 +583,7 @@ CssLogic.prototype = {
rule._matchId = this._matchId;
rule._passId = this._passId;
this._matchedRules.push([rule, status]);
this._matchedRules.push([rule, status, distance]);
}
// Add element.style information.
@ -584,8 +591,10 @@ CssLogic.prototype = {
const rule = new CssRule(null, { style: element.style }, element);
rule._matchId = this._matchId;
rule._passId = this._passId;
this._matchedRules.push([rule, status]);
this._matchedRules.push([rule, status, distance]);
}
distance--;
} while ((element = element.parentNode) &&
element.nodeType === nodeConstants.ELEMENT_NODE);
},
@ -720,7 +729,7 @@ CssLogic.href = function(sheet) {
function CssSheet(cssLogic, domSheet, index) {
this._cssLogic = cssLogic;
this.domSheet = domSheet;
this.index = this.contentSheet ? index : -100 * index;
this.index = this.authorSheet ? index : -100 * index;
// Cache of the sheets href. Cached by the getter.
this._href = null;
@ -738,23 +747,49 @@ function CssSheet(cssLogic, domSheet, index) {
CssSheet.prototype = {
_passId: null,
_contentSheet: null,
_agentSheet: null,
_authorSheet: null,
_userSheet: null,
/**
* Tells if the stylesheet is provided by the browser or not.
* Check if the stylesheet is an agent stylesheet (provided by the browser).
*
* @return {boolean} false if this is a browser-provided stylesheet, or true
* otherwise.
* @return {boolean} true if this is an agent stylesheet, false otherwise.
*/
get contentSheet() {
if (this._contentSheet === null) {
this._contentSheet = isContentStylesheet(this.domSheet);
get agentSheet() {
if (this._agentSheet === null) {
this._agentSheet = isAgentStylesheet(this.domSheet);
}
return this._contentSheet;
return this._agentSheet;
},
/**
* Tells if the stylesheet is disabled or not.
* Check if the stylesheet is an author stylesheet (provided by the content page).
*
* @return {boolean} true if this is an author stylesheet, false otherwise.
*/
get authorSheet() {
if (this._authorSheet === null) {
this._authorSheet = isAuthorStylesheet(this.domSheet);
}
return this._authorSheet;
},
/**
* Check if the stylesheet is a user stylesheet (provided by userChrome.css or
* userContent.css).
*
* @return {boolean} true if this is a user stylesheet, false otherwise.
*/
get userSheet() {
if (this._userSheet === null) {
this._userSheet = isUserStylesheet(this.domSheet);
}
return this._userSheet;
},
/**
* Check if the stylesheet is disabled or not.
* @return {boolean} true if this stylesheet is disabled, or false otherwise.
*/
get disabled() {
@ -803,7 +838,7 @@ CssSheet.prototype = {
this._sheetAllowed = true;
const filter = this._cssLogic.sourceFilter;
if (filter === FILTER.USER && !this.contentSheet) {
if (filter === FILTER.USER && !this.authorSheet) {
this._sheetAllowed = false;
}
if (filter !== FILTER.USER && filter !== FILTER.UA) {
@ -861,10 +896,7 @@ CssSheet.prototype = {
let ruleFound = false;
if (cacheId in this._rules) {
for (let i = 0, rulesLen = this._rules[cacheId].length;
i < rulesLen;
i++) {
rule = this._rules[cacheId][i];
for (rule of this._rules[cacheId]) {
if (rule.domRule === domRule) {
ruleFound = true;
break;
@ -921,13 +953,17 @@ function CssRule(cssSheet, domRule, element) {
this.source += " @media " + this.mediaText;
}
this.href = this._cssSheet.href;
this.contentRule = this._cssSheet.contentSheet;
this.authorRule = this._cssSheet.authorSheet;
this.userRule = this._cssSheet.userSheet;
this.agentRule = this._cssSheet.agentSheet;
} else if (element) {
this._selectors = [ new CssSelector(this, "@element.style", 0) ];
this.line = -1;
this.source = l10n("rule.sourceElement");
this.href = "#";
this.contentRule = true;
this.authorRule = true;
this.userRule = false;
this.agentRule = false;
this.sourceElement = element;
}
}
@ -1027,7 +1063,7 @@ CssRule.prototype = {
function CssSelector(cssRule, selector, index) {
this.cssRule = cssRule;
this.text = selector;
this.elementStyle = this.text == "@element.style";
this.inlineStyle = this.text == "@element.style";
this._specificity = null;
this.selectorIndex = index;
}
@ -1069,13 +1105,31 @@ CssSelector.prototype = {
},
/**
* Check if the selector comes from a browser-provided stylesheet.
* Check if the selector comes from an agent stylesheet (provided by the browser).
*
* @return {boolean} true if the selector comes from a content-provided
* stylesheet, or false otherwise.
* @return {boolean} true if this is an agent stylesheet, false otherwise.
*/
get contentRule() {
return this.cssRule.contentRule;
get agentRule() {
return this.cssRule.agentRule;
},
/**
* Check if the selector comes from an author stylesheet (provided by the content page).
*
* @return {boolean} true if this is an author stylesheet, false otherwise.
*/
get authorRule() {
return this.cssRule.authorRule;
},
/**
* Check if the selector comes from a user stylesheet (provided by userChrome.css or
* userContent.css).
*
* @return {boolean} true if this is a user stylesheet, false otherwise.
*/
get userRule() {
return this.cssRule.userRule;
},
/**
@ -1127,7 +1181,7 @@ CssSelector.prototype = {
* @return {Number} The selector's specificity.
*/
get specificity() {
if (this.elementStyle) {
if (this.inlineStyle) {
// We can't ask specificity from DOMUtils as element styles don't provide
// CSSStyleRule interface DOMUtils expect. However, specificity of element
// style is constant, 1,0,0,0 or 0x40000000, just return the constant
@ -1135,13 +1189,11 @@ CssSelector.prototype = {
return 0x40000000;
}
if (this._specificity) {
return this._specificity;
if (typeof this._specificity !== "number") {
this._specificity = InspectorUtils.getSpecificity(this.cssRule.domRule,
this.selectorIndex);
}
this._specificity = InspectorUtils.getSpecificity(this.cssRule.domRule,
this.selectorIndex);
return this._specificity;
},
@ -1232,11 +1284,6 @@ CssPropertyInfo.prototype = {
// Sort the selectors by how well they match the given element.
this._matchedSelectors.sort(function(selectorInfo1, selectorInfo2) {
if (selectorInfo1.status > selectorInfo2.status) {
return -1;
} else if (selectorInfo2.status > selectorInfo1.status) {
return 1;
}
return selectorInfo1.compareTo(selectorInfo2);
});
@ -1254,7 +1301,7 @@ CssPropertyInfo.prototype = {
* @param {CssSelector} selector the matched CssSelector object.
* @param {STATUS} status the CssSelector match status.
*/
_processMatchedSelector: function(selector, status) {
_processMatchedSelector: function(selector, status, distance) {
const cssRule = selector.cssRule;
const value = cssRule.getPropertyValue(this.property);
if (value &&
@ -1262,7 +1309,7 @@ CssPropertyInfo.prototype = {
(status == STATUS.PARENT_MATCH &&
this._isInherited(this.property)))) {
const selectorInfo = new CssSelectorInfo(selector, this.property, value,
status);
status, distance);
this._matchedSelectors.push(selectorInfo);
}
},
@ -1311,10 +1358,11 @@ CssPropertyInfo.prototype = {
* @param {STATUS} status The selector match status.
* @constructor
*/
function CssSelectorInfo(selector, property, value, status) {
function CssSelectorInfo(selector, property, value, status, distance) {
this.selector = selector;
this.property = property;
this.status = status;
this.distance = distance;
this.value = value;
const priority = this.selector.cssRule.getPropertyPriority(this.property);
this.important = (priority === "important");
@ -1358,8 +1406,8 @@ CssSelectorInfo.prototype = {
* @return {boolean} true if the CssSelector comes from element.style, or
* false otherwise.
*/
get elementStyle() {
return this.selector.elementStyle;
get inlineStyle() {
return this.selector.inlineStyle;
},
/**
@ -1418,74 +1466,152 @@ CssSelectorInfo.prototype = {
* @return {boolean} true if the selector comes from a browser-provided
* stylesheet, or false otherwise.
*/
get contentRule() {
return this.selector.contentRule;
get agentRule() {
return this.selector.agentRule;
},
/**
* Check if the selector comes from a webpage-provided stylesheet.
*
* @return {boolean} true if the selector comes from a webpage-provided
* stylesheet, or false otherwise.
*/
get authorRule() {
return this.selector.authorRule;
},
/**
* Check if the selector comes from a user stylesheet (userChrome.css or
* userContent.css).
*
* @return {boolean} true if the selector comes from a webpage-provided
* stylesheet, or false otherwise.
*/
get userRule() {
return this.selector.userRule;
},
/**
* Compare the current CssSelectorInfo instance to another instance, based on
* specificity information.
* the CSS cascade (see https://www.w3.org/TR/css-cascade-4/#cascading):
*
* @param {CssSelectorInfo} that The instance to compare ourselves against.
* @return number -1, 0, 1 depending on how that compares with this.
* The cascade sorts declarations according to the following criteria, in
* descending order of priority:
*
* - Rules targetting a node directly must always win over rules targetting an
* ancestor.
*
* - Origin and Importance
* The origin of a declaration is based on where it comes from and its
* importance is whether or not it is declared !important (see below). For
* our purposes here we can safely ignore Transition declarations and
* Animation declarations.
* The precedence of the various origins is, in descending order:
* - Transition declarations (ignored)
* - Important user agent declarations (User-Agent & !important)
* - Important user declarations (User & !important)
* - Important author declarations (Author & !important)
* - Animation declarations (ignored)
* - Normal author declarations (Author, normal weight)
* - Normal user declarations (User, normal weight)
* - Normal user agent declarations (User-Agent, normal weight)
*
* - Specificity (see https://www.w3.org/TR/selectors/#specificity)
* - A selectors specificity is calculated for a given element as follows:
* - count the number of ID selectors in the selector (= A)
* - count the number of class selectors, attributes selectors, and
* pseudo-classes in the selector (= B)
* - count the number of type selectors and pseudo-elements in the
* selector (= C)
* - ignore the universal selector
* - So "UL OL LI.red" has a specificity of a=0 b=1 c=3.
*
* - Order of Appearance
* - The last declaration in document order wins. For this purpose:
* - Declarations from imported style sheets are ordered as if their style
* sheets were substituted in place of the @import rule.
* - Declarations from style sheets independently linked by the
* originating document are treated as if they were concatenated in
* linking order, as determined by the host document language.
* - Declarations from style attributes are ordered according to the
* document order of the element the style attribute appears on, and are
* all placed after any style sheets.
* - We use three methods to calculate this:
* - Sheet index
* - Rule line
* - Rule column
*
* @param {CssSelectorInfo} that
* The instance to compare ourselves against.
* @return {Number}
* -1, 0, 1 depending on how that compares with this.
*/
compareTo: function(that) {
if (!this.contentRule && that.contentRule) {
return 1;
}
if (this.contentRule && !that.contentRule) {
return -1;
let current = null;
// Rules targetting the node must always win over rules targetting a node's
// ancestor.
current = this.compare(that, "distance", COMPAREMODE.INTEGER);
if (current) {
return current;
}
if (this.elementStyle && !that.elementStyle) {
if (!this.important && that.important) {
return 1;
if (this.important) {
// User-Agent & !important
// User & !important
// Author & !important
for (const propName of ["agentRule", "userRule", "authorRule"]) {
current = this.compare(that, propName, COMPAREMODE.BOOLEAN);
if (current) {
return current;
}
}
return -1;
}
if (!this.elementStyle && that.elementStyle) {
if (this.important && !that.important) {
return -1;
// Author, normal weight
// User, normal weight
// User-Agent, normal weight
for (const propName of ["authorRule", "userRule", "agentRule"]) {
current = this.compare(that, propName, COMPAREMODE.BOOLEAN);
if (current) {
return current;
}
return 1;
}
if (this.important && !that.important) {
return -1;
}
if (that.important && !this.important) {
return 1;
// Specificity
// Sheet index
// Rule line
// Rule column
for (const propName of ["specificity", "sheetIndex", "ruleLine", "ruleColumn"]) {
current = this.compare(that, propName, COMPAREMODE.INTEGER);
if (current) {
return current;
}
}
if (this.specificity > that.specificity) {
return -1;
}
if (that.specificity > this.specificity) {
return 1;
}
// A rule has been compared against itself so return 0.
return 0;
},
if (this.sheetIndex > that.sheetIndex) {
return -1;
compare: function(that, propertyName, type) {
switch (type) {
case COMPAREMODE.BOOLEAN:
if (this[propertyName] && !that[propertyName]) {
return -1;
}
if (!this[propertyName] && that[propertyName]) {
return 1;
}
break;
case COMPAREMODE.INTEGER:
if (this[propertyName] > that[propertyName]) {
return -1;
}
if (this[propertyName] < that[propertyName]) {
return 1;
}
break;
}
if (that.sheetIndex > this.sheetIndex) {
return 1;
}
if (this.ruleLine > that.ruleLine) {
return -1;
}
if (that.ruleLine > this.ruleLine) {
return 1;
}
if (this.ruleColumn > that.ruleColumn) {
return -1;
}
if (that.ruleColumn > this.ruleColumn) {
return 1;
}
return 0;
},

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

@ -483,7 +483,7 @@ var PageStyleActor = protocol.ActorClassWithSpec(pageStyleSpec, {
// node.
getSelectorSource: function(selectorInfo, relativeTo) {
let result = selectorInfo.selector.text;
if (selectorInfo.elementStyle) {
if (selectorInfo.inlineStyle) {
const source = selectorInfo.sourceElement;
if (source === relativeTo) {
result = "this";
@ -642,7 +642,7 @@ var PageStyleActor = protocol.ActorClassWithSpec(pageStyleSpec, {
for (let i = domRules.length - 1; i >= 0; i--) {
const domRule = domRules[i];
const isSystem = !SharedCssLogic.isContentStylesheet(domRule.parentStyleSheet);
const isSystem = !SharedCssLogic.isAuthorStylesheet(domRule.parentStyleSheet);
if (isSystem && options.filter != SharedCssLogic.FILTER.UA) {
continue;

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

@ -398,7 +398,7 @@ var StyleSheetActor = protocol.ActorClassWithSpec(styleSheetSpec, {
nodeHref: docHref,
disabled: this.rawSheet.disabled,
title: this.rawSheet.title,
system: !CssLogic.isContentStylesheet(this.rawSheet),
system: !CssLogic.isAuthorStylesheet(this.rawSheet),
styleSheetIndex: this.styleSheetIndex,
sourceMapURL: this.rawSheet.sourceMapURL,
};

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

@ -112,14 +112,36 @@ exports.CSSRuleTypeName = {
exports.l10n = name => styleInspectorL10N.getStr(name);
/**
* Is the given property sheet a content stylesheet?
* Is the given property sheet an author stylesheet?
*
* @param {CSSStyleSheet} sheet a stylesheet
* @return {boolean} true if the given stylesheet is a content stylesheet,
* @return {boolean} true if the given stylesheet is an author stylesheet,
* false otherwise.
*/
exports.isContentStylesheet = function(sheet) {
return sheet.parsingMode !== "agent";
exports.isAuthorStylesheet = function(sheet) {
return sheet.parsingMode === "author";
};
/**
* Is the given property sheet a user stylesheet?
*
* @param {CSSStyleSheet} sheet a stylesheet
* @return {boolean} true if the given stylesheet is a user stylesheet,
* false otherwise.
*/
exports.isUserStylesheet = function(sheet) {
return sheet.parsingMode === "user";
};
/**
* Is the given property sheet a agent stylesheet?
*
* @param {CSSStyleSheet} sheet a stylesheet
* @return {boolean} true if the given stylesheet is a agent stylesheet,
* false otherwise.
*/
exports.isAgentStylesheet = function(sheet) {
return sheet.parsingMode === "agent";
};
/**