Skip to content

Commit a853f7e

Browse files
pfaffeDevtools-frontend LUCI CQ
authored andcommitted
[css value tracing] Reland: evaluate percentages in longhands
This adds support for evaluating <percentage> units in css value tracing, but only for longhands. For shorthands, we need additional reasoning about which longhand the unit pertains to in order to understand whether it's relative to a width or a height. Bug: 401213719 Change-Id: I5da7dfba1175e10c9c2f641f7c396cba73a7f5ff Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6425300 Commit-Queue: Philip Pfaffe <[email protected]> Auto-Submit: Philip Pfaffe <[email protected]> Reviewed-by: Eric Leese <[email protected]> Feels: Philip Pfaffe <[email protected]> Commit-Queue: Eric Leese <[email protected]>
1 parent e2e4402 commit a853f7e

File tree

5 files changed

+68
-27
lines changed

5 files changed

+68
-27
lines changed

front_end/core/sdk/CSSModel.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import * as Root from '../root/root.js';
4343
import {CSSFontFace} from './CSSFontFace.js';
4444
import {CSSMatchedStyles} from './CSSMatchedStyles.js';
4545
import {CSSMedia} from './CSSMedia.js';
46+
import {cssMetadata} from './CSSMetadata.js';
4647
import {CSSStyleRule} from './CSSRule.js';
4748
import {CSSStyleDeclaration, Type} from './CSSStyleDeclaration.js';
4849
import {CSSStyleSheetHeader} from './CSSStyleSheetHeader.js';
@@ -117,9 +118,16 @@ export class CSSModel extends SDKModel<EventTypes> {
117118
return this.#colorScheme;
118119
}
119120

120-
async resolveValues(nodeId: Protocol.DOM.NodeId, ...values: string[]): Promise<string[]|null> {
121-
const response = await this.agent.invoke_resolveValues({values, nodeId});
122-
return response.getError() ? null : response.results;
121+
async resolveValues(propertyName: string|undefined, nodeId: Protocol.DOM.NodeId, ...values: string[]):
122+
Promise<string[]|null> {
123+
if (propertyName && cssMetadata().getLonghands(propertyName)?.length) {
124+
return null;
125+
}
126+
const response = await this.agent.invoke_resolveValues({values, nodeId, propertyName});
127+
if (response.getError()) {
128+
return null;
129+
}
130+
return response.results;
123131
}
124132

125133
headersForSourceURL(sourceURL: Platform.DevToolsPath.UrlString): CSSStyleSheetHeader[] {

front_end/core/sdk/CSSPropertyParserMatchers.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -685,10 +685,10 @@ export class LengthMatch implements Match {
685685
export class LengthMatcher extends matcherBase(LengthMatch) {
686686
// clang-format on
687687
static readonly LENGTH_UNITS = new Set([
688-
'em', 'ex', 'ch', 'cap', 'ic', 'lh', 'rem', 'rex', 'rch', 'rlh', 'ric', 'rcap', 'pt',
689-
'pc', 'in', 'cm', 'mm', 'Q', 'vw', 'vh', 'vi', 'vb', 'vmin', 'vmax', 'dvw', 'dvh',
690-
'dvi', 'dvb', 'dvmin', 'dvmax', 'svw', 'svh', 'svi', 'svb', 'svmin', 'svmax', 'lvw', 'lvh', 'lvi',
691-
'lvb', 'lvmin', 'lvmax', 'cqw', 'cqh', 'cqi', 'cqb', 'cqmin', 'cqmax', 'cqem', 'cqlh', 'cqex', 'cqch',
688+
'em', 'ex', 'ch', 'cap', 'ic', 'lh', 'rem', 'rex', 'rch', 'rlh', 'ric', 'rcap', 'pt', 'pc',
689+
'in', 'cm', 'mm', 'Q', 'vw', 'vh', 'vi', 'vb', 'vmin', 'vmax', 'dvw', 'dvh', 'dvi', 'dvb',
690+
'dvmin', 'dvmax', 'svw', 'svh', 'svi', 'svb', 'svmin', 'svmax', 'lvw', 'lvh', 'lvi', 'lvb', 'lvmin', 'lvmax',
691+
'cqw', 'cqh', 'cqi', 'cqb', 'cqmin', 'cqmax', 'cqem', 'cqlh', 'cqex', 'cqch', '%'
692692
]);
693693
override matches(node: CodeMirror.SyntaxNode, matching: BottomUpTreeMatching): LengthMatch|null {
694694
if (node.name !== 'NumberLiteral') {

front_end/panels/elements/CSSValueTraceView.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ async function showTrace(
7373
view.showTrace(
7474
property, null, matchedStyles, new Map(),
7575
Elements.StylePropertyTreeElement.getPropertyRenderers(
76-
property.ownerStyle, treeElement.parentPane(), matchedStyles, treeElement,
76+
property.name, property.ownerStyle, treeElement.parentPane(), matchedStyles, treeElement,
7777
treeElement.getComputedStyles() ?? new Map()));
7878
return await viewFunction.nextInput;
7979
}

front_end/panels/elements/StylePropertyTreeElement.test.ts

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
286286
const {valueElement} = Elements.PropertyRenderer.Renderer.renderValueElement(
287287
property, matchedResult,
288288
Elements.StylePropertyTreeElement.getPropertyRenderers(
289-
matchedStyles.nodeStyles()[0], stylesSidebarPane, matchedStyles, null, new Map()),
289+
property.name, matchedStyles.nodeStyles()[0], stylesSidebarPane, matchedStyles, null, new Map()),
290290
context);
291291

292292
const colorSwatch = valueElement.querySelector('devtools-color-swatch');
@@ -1657,7 +1657,7 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
16571657
const addPopoverPromise = Promise.withResolvers<void>();
16581658
sinon.stub(Elements.StylePropertyTreeElement.LengthRenderer.prototype, 'popOverAttachedForTest')
16591659
.callsFake(() => addPopoverPromise.resolve());
1660-
const stylePropertyTreeElement = getTreeElement('margin', '5px 2em');
1660+
const stylePropertyTreeElement = getTreeElement('property', '5px 2em');
16611661
setMockConnectionResponseHandler('CSS.getComputedStyleForNode', () => ({computedStyle: {}}));
16621662

16631663
await stylePropertyTreeElement.onpopulate();
@@ -1666,6 +1666,17 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
16661666
const popover = stylePropertyTreeElement.valueElement?.querySelector('devtools-tooltip');
16671667
assert.strictEqual(popover?.innerText, '15px');
16681668
});
1669+
1670+
it('passes the property name to evaluations', async () => {
1671+
const cssModel = stylesSidebarPane.cssModel();
1672+
assert.exists(cssModel);
1673+
const resolveValuesStub = sinon.stub(cssModel, 'resolveValues').resolves([]);
1674+
const stylePropertyTreeElement = getTreeElement('left', '2%');
1675+
stylePropertyTreeElement.updateTitle();
1676+
1677+
assert.isTrue(resolveValuesStub.calledOnce);
1678+
assert.strictEqual(resolveValuesStub.args[0][0], 'left');
1679+
});
16691680
});
16701681

16711682
describe('MathFunctionRenderer', () => {
@@ -1718,13 +1729,29 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
17181729
view.showTrace(
17191730
property, null, matchedStyles, new Map(),
17201731
Elements.StylePropertyTreeElement.getPropertyRenderers(
1721-
property.ownerStyle, stylesSidebarPane, matchedStyles, null, new Map()));
1732+
property.name, property.ownerStyle, stylesSidebarPane, matchedStyles, null, new Map()));
17221733

17231734
assert.isTrue(evaluationSpy.calledOnce);
17241735
const originalText = evaluationSpy.args[0][0].textContent;
17251736
await evaluationSpy.returnValues[0];
17261737
assert.strictEqual(originalText, evaluationSpy.args[0][0].textContent);
17271738
});
1739+
1740+
it('shows the original text during tracing when evaluation fails', async () => {
1741+
const cssModel = stylesSidebarPane.cssModel();
1742+
assert.exists(cssModel);
1743+
const resolveValuesStub = sinon.stub(cssModel, 'resolveValues').resolves([]);
1744+
const property = addProperty('width', 'calc(1 + 1)');
1745+
1746+
const view = new Elements.CSSValueTraceView.CSSValueTraceView(undefined, () => {});
1747+
view.showTrace(
1748+
property, null, matchedStyles, new Map(),
1749+
Elements.StylePropertyTreeElement.getPropertyRenderers(
1750+
property.name, property.ownerStyle, stylesSidebarPane, matchedStyles, null, new Map()));
1751+
1752+
assert.isTrue(resolveValuesStub.calledOnce);
1753+
assert.strictEqual(resolveValuesStub.args[0][0], 'width');
1754+
});
17281755
});
17291756

17301757
describe('AutoBaseRenderer', () => {

front_end/panels/elements/StylePropertyTreeElement.ts

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,8 @@ export class VariableRenderer extends rendererBase(SDK.CSSPropertyParserMatchers
262262
const {nodes, cssControls} = Renderer.renderValueNodes(
263263
{name: declaration.name, value: declaration.value ?? ''},
264264
substitution.cachedParsedValue(declaration.declaration, this.#matchedStyles, this.#computedStyles),
265-
getPropertyRenderers(declaration.style, this.#stylesPane, this.#matchedStyles, null, this.#computedStyles),
265+
getPropertyRenderers(
266+
declaration.name, declaration.style, this.#stylesPane, this.#matchedStyles, null, this.#computedStyles),
266267
substitution);
267268
cssControls.forEach((value, key) => value.forEach(control => context.addControl(key, control)));
268269
return nodes;
@@ -658,7 +659,7 @@ export class ColorMixRenderer extends rendererBase(SDK.CSSPropertyParserMatchers
658659
context.addControl('color', swatch);
659660
const nodeId = this.#pane.node()?.id;
660661
if (nodeId !== undefined) {
661-
void this.#pane.cssModel()?.resolveValues(nodeId, colorMixText).then(results => {
662+
void this.#pane.cssModel()?.resolveValues(undefined, nodeId, colorMixText).then(results => {
662663
if (results) {
663664
const color = Common.Color.parse(results[0]);
664665
if (color) {
@@ -1258,10 +1259,12 @@ export class LengthRenderer extends rendererBase(SDK.CSSPropertyParserMatchers.L
12581259
// clang-format on
12591260
readonly #stylesPane: StylesSidebarPane;
12601261
readonly #treeElement: StylePropertyTreeElement|null;
1261-
constructor(stylesPane: StylesSidebarPane, treeElement: StylePropertyTreeElement|null) {
1262+
readonly #propertyName: string;
1263+
constructor(stylesPane: StylesSidebarPane, propertyName: string, treeElement: StylePropertyTreeElement|null) {
12621264
super();
12631265
this.#stylesPane = stylesPane;
12641266
this.#treeElement = treeElement;
1267+
this.#propertyName = propertyName;
12651268
}
12661269

12671270
override render(match: SDK.CSSPropertyParserMatchers.LengthMatch, context: RenderingContext): Node[] {
@@ -1284,7 +1287,7 @@ export class LengthRenderer extends rendererBase(SDK.CSSPropertyParserMatchers.L
12841287
return;
12851288
}
12861289

1287-
const pixelValue = await this.#stylesPane.cssModel()?.resolveValues(nodeId, value);
1290+
const pixelValue = await this.#stylesPane.cssModel()?.resolveValues(this.#propertyName, nodeId, value);
12881291

12891292
if (pixelValue) {
12901293
valueElement.textContent = pixelValue[0];
@@ -1297,7 +1300,7 @@ export class LengthRenderer extends rendererBase(SDK.CSSPropertyParserMatchers.L
12971300
return;
12981301
}
12991302

1300-
const pixelValue = await this.#stylesPane.cssModel()?.resolveValues(nodeId, value);
1303+
const pixelValue = await this.#stylesPane.cssModel()?.resolveValues(this.#propertyName, nodeId, value);
13011304
if (!pixelValue) {
13021305
return;
13031306
}
@@ -1324,14 +1327,16 @@ export class MathFunctionRenderer extends rendererBase(SDK.CSSPropertyParserMatc
13241327
readonly #matchedStyles: SDK.CSSMatchedStyles.CSSMatchedStyles;
13251328
readonly #computedStyles: Map<string, string>;
13261329
readonly #treeElement: StylePropertyTreeElement|null;
1330+
readonly #propertyName: string;
13271331
constructor(
13281332
stylesPane: StylesSidebarPane, matchedStyles: SDK.CSSMatchedStyles.CSSMatchedStyles,
1329-
computedStyles: Map<string, string>, treeElement: StylePropertyTreeElement|null) {
1333+
computedStyles: Map<string, string>, propertyName: string, treeElement: StylePropertyTreeElement|null) {
13301334
super();
13311335
this.#matchedStyles = matchedStyles;
13321336
this.#computedStyles = computedStyles;
13331337
this.#stylesPane = stylesPane;
13341338
this.#treeElement = treeElement;
1339+
this.#propertyName = propertyName;
13351340
}
13361341

13371342
override render(match: SDK.CSSPropertyParserMatchers.MathFunctionMatch, context: RenderingContext): Node[] {
@@ -1367,7 +1372,7 @@ export class MathFunctionRenderer extends rendererBase(SDK.CSSPropertyParserMatc
13671372
if (nodeId === undefined) {
13681373
return;
13691374
}
1370-
const evaled = await this.#stylesPane.cssModel()?.resolveValues(nodeId, value);
1375+
const evaled = await this.#stylesPane.cssModel()?.resolveValues(this.#propertyName, nodeId, value);
13711376
if (!evaled?.[0] || evaled[0] === value) {
13721377
return;
13731378
}
@@ -1383,7 +1388,7 @@ export class MathFunctionRenderer extends rendererBase(SDK.CSSPropertyParserMatc
13831388
// and compare the function result to the values of all its arguments. Evaluating the arguments eliminates nested
13841389
// function calls and normalizes all units to px.
13851390
values.unshift(functionText);
1386-
const evaledArgs = await this.#stylesPane.cssModel()?.resolveValues(nodeId, ...values);
1391+
const evaledArgs = await this.#stylesPane.cssModel()?.resolveValues(this.#propertyName, nodeId, ...values);
13871392
if (!evaledArgs) {
13881393
return;
13891394
}
@@ -1525,7 +1530,7 @@ export class PositionTryRenderer extends rendererBase(SDK.CSSPropertyParserMatch
15251530
}
15261531

15271532
export function getPropertyRenderers(
1528-
style: SDK.CSSStyleDeclaration.CSSStyleDeclaration, stylesPane: StylesSidebarPane,
1533+
propertyName: string, style: SDK.CSSStyleDeclaration.CSSStyleDeclaration, stylesPane: StylesSidebarPane,
15291534
matchedStyles: SDK.CSSMatchedStyles.CSSMatchedStyles, treeElement: StylePropertyTreeElement|null,
15301535
computedStyles: Map<string, string>): Array<MatchRenderer<SDK.CSSPropertyParser.Match>> {
15311536
return [
@@ -1546,8 +1551,8 @@ export function getPropertyRenderers(
15461551
new PositionAnchorRenderer(stylesPane),
15471552
new FlexGridRenderer(stylesPane, treeElement),
15481553
new PositionTryRenderer(matchedStyles),
1549-
new LengthRenderer(stylesPane, treeElement),
1550-
new MathFunctionRenderer(stylesPane, matchedStyles, computedStyles, treeElement),
1554+
new LengthRenderer(stylesPane, propertyName, treeElement),
1555+
new MathFunctionRenderer(stylesPane, matchedStyles, computedStyles, propertyName, treeElement),
15511556
new AutoBaseRenderer(computedStyles),
15521557
new BinOpRenderer(),
15531558
];
@@ -1951,10 +1956,11 @@ export class StylePropertyTreeElement extends UI.TreeOutline.TreeElement {
19511956
this.expandElement.setAttribute('jslog', `${VisualLogging.expand().track({click: true})}`);
19521957
}
19531958

1954-
const renderers = this.property.parsedOk ? getPropertyRenderers(
1955-
this.style, this.parentPaneInternal, this.matchedStylesInternal,
1956-
this, this.getComputedStyles() ?? new Map()) :
1957-
[];
1959+
const renderers = this.property.parsedOk ?
1960+
getPropertyRenderers(
1961+
this.name, this.style, this.parentPaneInternal, this.matchedStylesInternal, this,
1962+
this.getComputedStyles() ?? new Map()) :
1963+
[];
19581964

19591965
if (Root.Runtime.experiments.isEnabled('font-editor') && this.property.parsedOk) {
19601966
renderers.push(new FontRenderer(this));
@@ -2118,7 +2124,7 @@ export class StylePropertyTreeElement extends UI.TreeOutline.TreeElement {
21182124
?.getWidget()
21192125
?.showTrace(
21202126
property, text, matchedStyles, computedStyles,
2121-
getPropertyRenderers(
2127+
getPropertyRenderers(property.name,
21222128
property.ownerStyle, stylesPane, matchedStyles, null, computedStyles));
21232129
}
21242130
}}

0 commit comments

Comments
 (0)