Skip to content

Commit af77ebb

Browse files
connorjclarkconnorjclark
authored
core(font-size): use order from protocol as implicit specificity (#13501)
1 parent 8c61b74 commit af77ebb

File tree

2 files changed

+20
-118
lines changed

2 files changed

+20
-118
lines changed

‎core/gather/gatherers/seo/font-size.js

Lines changed: 9 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -34,38 +34,6 @@ function hasFontSizeDeclaration(style) {
3434
return !!style && !!style.cssProperties.find(({name}) => name === FONT_SIZE_PROPERTY_NAME);
3535
}
3636

37-
/**
38-
* Computes the CSS specificity of a given selector, i.e. #id > .class > div
39-
* TODO: Handle pseudo selectors (:not(), :where, :nth-child) and attribute selectors
40-
* LIMITATION: !important is not respected
41-
*
42-
* @see https://developer.mozilla.org/en-US/docs/Web/CSS/Specificity
43-
* @see https://www.smashingmagazine.com/2010/04/css-specificity-and-inheritance/
44-
* @see https://drafts.csswg.org/selectors-4/#specificity-rules
45-
*
46-
* @param {string} selector
47-
* @return {number}
48-
*/
49-
function computeSelectorSpecificity(selector) {
50-
// Remove universal selector and separator characters, then split.
51-
const tokens = selector.replace(/[*\s+>~]/g, ' ').split(' ');
52-
53-
let numIDs = 0;
54-
let numClasses = 0;
55-
let numTypes = 0;
56-
57-
for (const token of tokens) {
58-
const ids = token.match(/(\b|^)#[a-z0-9_-]+/gi) || [];
59-
const classes = token.match(/(\b|^)\.[a-z0-9_-]+/gi) || [];
60-
const types = token.match(/^[a-z]+/i) ? [1] : [];
61-
numIDs += ids.length;
62-
numClasses += classes.length;
63-
numTypes += types.length;
64-
}
65-
66-
return Math.min(9, numIDs) * 100 + Math.min(9, numClasses) * 10 + Math.min(9, numTypes);
67-
}
68-
6937
/**
7038
* Finds the most specific directly matched CSS font-size rule from the list.
7139
*
@@ -74,31 +42,21 @@ function computeSelectorSpecificity(selector) {
7442
* @return {NodeFontData['cssRule']|undefined}
7543
*/
7644
function findMostSpecificMatchedCSSRule(matchedCSSRules = [], isDeclarationOfInterest) {
77-
let maxSpecificity = -Infinity;
78-
/** @type {LH.Crdp.CSS.CSSRule|undefined} */
79-
let maxSpecificityRule;
80-
81-
for (const {rule, matchingSelectors} of matchedCSSRules) {
82-
if (isDeclarationOfInterest(rule.style)) {
83-
const specificities = matchingSelectors.map(idx =>
84-
computeSelectorSpecificity(rule.selectorList.selectors[idx].text)
85-
);
86-
const specificity = Math.max(...specificities);
87-
// Use greater OR EQUAL so that the last rule wins in the event of a tie
88-
if (specificity >= maxSpecificity) {
89-
maxSpecificity = specificity;
90-
maxSpecificityRule = rule;
91-
}
45+
let mostSpecificRule;
46+
for (let i = matchedCSSRules.length - 1; i >= 0; i--) {
47+
if (isDeclarationOfInterest(matchedCSSRules[i].rule.style)) {
48+
mostSpecificRule = matchedCSSRules[i].rule;
49+
break;
9250
}
9351
}
9452

95-
if (maxSpecificityRule) {
53+
if (mostSpecificRule) {
9654
return {
9755
type: 'Regular',
98-
...maxSpecificityRule.style,
56+
...mostSpecificRule.style,
9957
parentRule: {
100-
origin: maxSpecificityRule.origin,
101-
selectors: maxSpecificityRule.selectorList.selectors,
58+
origin: mostSpecificRule.origin,
59+
selectors: mostSpecificRule.selectorList.selectors,
10260
},
10361
};
10462
}
@@ -376,7 +334,6 @@ class FontSize extends FRGatherer {
376334

377335
export default FontSize;
378336
export {
379-
computeSelectorSpecificity,
380337
getEffectiveFontRule,
381338
findMostSpecificMatchedCSSRule,
382339
};

‎core/test/gather/gatherers/seo/font-size-test.js

Lines changed: 11 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,7 @@
66

77
import assert from 'assert/strict';
88

9-
import FontSizeGather, {
10-
computeSelectorSpecificity, getEffectiveFontRule,
11-
} from '../../../../gather/gatherers/seo/font-size.js';
9+
import FontSizeGather, {getEffectiveFontRule} from '../../../../gather/gatherers/seo/font-size.js';
1210

1311
let fontSizeGather;
1412

@@ -178,59 +176,6 @@ describe('Font size gatherer', () => {
178176
});
179177
});
180178

181-
describe('#computeSelectorSpecificity', () => {
182-
const compute = computeSelectorSpecificity;
183-
184-
it('should handle basic selectors', () => {
185-
expect(compute('h1')).toEqual(1);
186-
expect(compute('h1 > p > span')).toEqual(3);
187-
});
188-
189-
it('should handle class selectors', () => {
190-
expect(compute('h1.foo')).toEqual(11);
191-
expect(compute('.foo')).toEqual(10);
192-
expect(compute('h1 > p.other.yeah > span')).toEqual(23);
193-
});
194-
195-
it('should handle ID selectors', () => {
196-
expect(compute('h1#awesome.foo')).toEqual(111);
197-
expect(compute('#awesome.foo')).toEqual(110);
198-
expect(compute('#awesome')).toEqual(100);
199-
expect(compute('h1 > p.other > span#the-text')).toEqual(113);
200-
});
201-
202-
it('should ignore the univeral selector', () => {
203-
expect(compute('.foo')).toEqual(10);
204-
expect(compute('* .foo')).toEqual(10);
205-
expect(compute('.foo *')).toEqual(10);
206-
});
207-
208-
// Examples https://drafts.csswg.org/selectors-3/#specificity
209-
it('should handle l3 spec selectors', () => {
210-
expect(compute('*')).toEqual(0);
211-
expect(compute('LI')).toEqual(1);
212-
expect(compute('UL LI')).toEqual(2);
213-
expect(compute('UL OL+LI')).toEqual(3);
214-
// expect(compute('H1 + *[REL=up]')).toEqual(11); // TODO: Handle attribute selectors
215-
expect(compute('UL OL LI.red')).toEqual(13);
216-
expect(compute('LI.red.level')).toEqual(21);
217-
expect(compute('#x34y')).toEqual(100);
218-
// expect(compute('#s12:not(FOO)')).toEqual(101); // TODO: Handle pseudo selectors
219-
});
220-
221-
// Examples from https://drafts.csswg.org/selectors-4/#specificity-rules
222-
it('should handle l4 spec selectors', () => {
223-
expect(compute(':is(em, #foo)')).toEqual(100);
224-
// expect(compute('.qux:where(em, #foo#bar#baz)')).toEqual(10); // TODO: Handle pseudo selectors
225-
// expect(compute(':nth-child(even of li, .item)')).toEqual(20); // TODO: Handle pseudo selectors
226-
// expect(compute(':not(em, strong#foo)')).toEqual(101); // TODO: Handle pseudo selectors
227-
});
228-
229-
it('should cap the craziness', () => {
230-
expect(compute('h1.a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p')).toEqual(91);
231-
});
232-
});
233-
234179
describe('#getEffectiveFontRule', () => {
235180
const createProps = props => Object.entries(props).map(([name, value]) => ({name, value}));
236181
const createStyle = ({properties, id}) => ({
@@ -366,29 +311,29 @@ describe('Font size gatherer', () => {
366311
style: createStyle({id: 1, properties: {'font-size': '1em'}}),
367312
});
368313

369-
// Two matching selectors where 2nd is the global most specific, ID + class
314+
// Just ID
370315
const fontRuleB = createRule({
371316
origin: 'regular',
372-
selectors: ['html body *', '#main.foo'],
373-
style: createStyle({id: 2, properties: {'font-size': '2em'}}),
317+
selectors: ['#main'],
318+
style: createStyle({id: 2, properties: {'font-size': '3em'}}),
374319
});
375320

376-
// Just ID
321+
// Two matching selectors where 2nd is the global most specific, ID + class
377322
const fontRuleC = createRule({
378323
origin: 'regular',
379-
selectors: ['#main'],
380-
style: createStyle({id: 3, properties: {'font-size': '3em'}}),
324+
selectors: ['html body *', '#main.foo'],
325+
style: createStyle({id: 3, properties: {'font-size': '2em'}}),
381326
});
382327

383328
const matchedCSSRules = [
384329
{rule: fontRuleA, matchingSelectors: [0]},
385-
{rule: fontRuleB, matchingSelectors: [0, 1]},
386-
{rule: fontRuleC, matchingSelectors: [0]},
330+
{rule: fontRuleB, matchingSelectors: [0]},
331+
{rule: fontRuleC, matchingSelectors: [0, 1]},
387332
];
388333

389334
const result = getEffectiveFontRule({matchedCSSRules});
390-
// fontRuleB should have one for ID + class
391-
expect(result.styleSheetId).toEqual(2);
335+
// fontRuleC should have one for ID + class
336+
expect(result.styleSheetId).toEqual(3);
392337
});
393338

394339
it('should break ties with last one declared', () => {

0 commit comments

Comments
 (0)