From a0c4c4a06cc578e4b341bc4cf5c7950ed736b742 Mon Sep 17 00:00:00 2001 From: Philip Pfaffe Date: Mon, 24 Mar 2025 13:08:18 +0000 Subject: [PATCH] [css value tracing] evaluate percentages in longhands This adds support for evaluating 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: I823ef9d52bb12ab40e7bcabce770733ad36a51b7 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6376104 Reviewed-by: Eric Leese Auto-Submit: Philip Pfaffe Commit-Queue: Eric Leese --- front_end/core/sdk/CSSModel.ts | 14 ++++-- .../core/sdk/CSSPropertyParserMatchers.ts | 8 ++-- .../panels/elements/CSSValueTraceView.test.ts | 2 +- .../elements/StylePropertyTreeElement.test.ts | 33 +++++++++++-- .../elements/StylePropertyTreeElement.ts | 46 +++++++++++-------- 5 files changed, 72 insertions(+), 31 deletions(-) diff --git a/front_end/core/sdk/CSSModel.ts b/front_end/core/sdk/CSSModel.ts index ca93d5ced1..d09b276310 100644 --- a/front_end/core/sdk/CSSModel.ts +++ b/front_end/core/sdk/CSSModel.ts @@ -43,6 +43,7 @@ import * as Root from '../root/root.js'; import {CSSFontFace} from './CSSFontFace.js'; import {CSSMatchedStyles} from './CSSMatchedStyles.js'; import {CSSMedia} from './CSSMedia.js'; +import {cssMetadata} from './CSSMetadata.js'; import {CSSStyleRule} from './CSSRule.js'; import {CSSStyleDeclaration, Type} from './CSSStyleDeclaration.js'; import {CSSStyleSheetHeader} from './CSSStyleSheetHeader.js'; @@ -132,9 +133,16 @@ export class CSSModel extends SDKModel { return this.#colorScheme; } - async resolveValues(nodeId: Protocol.DOM.NodeId, ...values: string[]): Promise { - const response = await this.agent.invoke_resolveValues({values, nodeId}); - return response.getError() ? null : response.results; + async resolveValues(propertyName: string|undefined, nodeId: Protocol.DOM.NodeId, ...values: string[]): + Promise { + if (propertyName && cssMetadata().getLonghands(propertyName)?.length) { + return null; + } + const response = await this.agent.invoke_resolveValues({values, nodeId, propertyName}); + if (response.getError()) { + return null; + } + return response.results; } headersForSourceURL(sourceURL: Platform.DevToolsPath.UrlString): CSSStyleSheetHeader[] { diff --git a/front_end/core/sdk/CSSPropertyParserMatchers.ts b/front_end/core/sdk/CSSPropertyParserMatchers.ts index 38aaa02399..b40d009426 100644 --- a/front_end/core/sdk/CSSPropertyParserMatchers.ts +++ b/front_end/core/sdk/CSSPropertyParserMatchers.ts @@ -684,10 +684,10 @@ export class LengthMatch implements Match { export class LengthMatcher extends matcherBase(LengthMatch) { // clang-format on static readonly LENGTH_UNITS = new Set([ - 'em', 'ex', 'ch', 'cap', 'ic', 'lh', 'rem', 'rex', 'rch', 'rlh', 'ric', 'rcap', 'pt', - 'pc', 'in', 'cm', 'mm', 'Q', 'vw', 'vh', 'vi', 'vb', 'vmin', 'vmax', 'dvw', 'dvh', - 'dvi', 'dvb', 'dvmin', 'dvmax', 'svw', 'svh', 'svi', 'svb', 'svmin', 'svmax', 'lvw', 'lvh', 'lvi', - 'lvb', 'lvmin', 'lvmax', 'cqw', 'cqh', 'cqi', 'cqb', 'cqmin', 'cqmax', 'cqem', 'cqlh', 'cqex', 'cqch', + 'em', 'ex', 'ch', 'cap', 'ic', 'lh', 'rem', 'rex', 'rch', 'rlh', 'ric', 'rcap', 'pt', 'pc', + 'in', 'cm', 'mm', 'Q', 'vw', 'vh', 'vi', 'vb', 'vmin', 'vmax', 'dvw', 'dvh', 'dvi', 'dvb', + 'dvmin', 'dvmax', 'svw', 'svh', 'svi', 'svb', 'svmin', 'svmax', 'lvw', 'lvh', 'lvi', 'lvb', 'lvmin', 'lvmax', + 'cqw', 'cqh', 'cqi', 'cqb', 'cqmin', 'cqmax', 'cqem', 'cqlh', 'cqex', 'cqch', '%' ]); override matches(node: CodeMirror.SyntaxNode, matching: BottomUpTreeMatching): LengthMatch|null { if (node.name !== 'NumberLiteral') { diff --git a/front_end/panels/elements/CSSValueTraceView.test.ts b/front_end/panels/elements/CSSValueTraceView.test.ts index 322486ef7e..28495535ef 100644 --- a/front_end/panels/elements/CSSValueTraceView.test.ts +++ b/front_end/panels/elements/CSSValueTraceView.test.ts @@ -73,7 +73,7 @@ async function showTrace( view.showTrace( property, null, matchedStyles, new Map(), Elements.StylePropertyTreeElement.getPropertyRenderers( - property.ownerStyle, treeElement.parentPane(), matchedStyles, treeElement, + property.name, property.ownerStyle, treeElement.parentPane(), matchedStyles, treeElement, treeElement.getComputedStyles() ?? new Map())); return await viewFunction.nextInput; } diff --git a/front_end/panels/elements/StylePropertyTreeElement.test.ts b/front_end/panels/elements/StylePropertyTreeElement.test.ts index 24d89d4366..3fbb7cdd88 100644 --- a/front_end/panels/elements/StylePropertyTreeElement.test.ts +++ b/front_end/panels/elements/StylePropertyTreeElement.test.ts @@ -286,7 +286,7 @@ describeWithMockConnection('StylePropertyTreeElement', () => { const {valueElement} = Elements.PropertyRenderer.Renderer.renderValueElement( property, matchedResult, Elements.StylePropertyTreeElement.getPropertyRenderers( - matchedStyles.nodeStyles()[0], stylesSidebarPane, matchedStyles, null, new Map()), + property.name, matchedStyles.nodeStyles()[0], stylesSidebarPane, matchedStyles, null, new Map()), context); const colorSwatch = valueElement.querySelector('devtools-color-swatch'); @@ -1655,7 +1655,7 @@ describeWithMockConnection('StylePropertyTreeElement', () => { const addPopoverPromise = Promise.withResolvers(); sinon.stub(Elements.StylePropertyTreeElement.LengthRenderer.prototype, 'popOverAttachedForTest') .callsFake(() => addPopoverPromise.resolve()); - const stylePropertyTreeElement = getTreeElement('margin', '5px 2em'); + const stylePropertyTreeElement = getTreeElement('property', '5px 2em'); setMockConnectionResponseHandler('CSS.getComputedStyleForNode', () => ({computedStyle: {}})); await stylePropertyTreeElement.onpopulate(); @@ -1664,6 +1664,17 @@ describeWithMockConnection('StylePropertyTreeElement', () => { const popover = stylePropertyTreeElement.valueElement?.querySelector('devtools-tooltip'); assert.strictEqual(popover?.innerText, '15px'); }); + + it('passes the property name to evaluations', async () => { + const cssModel = stylesSidebarPane.cssModel(); + assert.exists(cssModel); + const resolveValuesStub = sinon.stub(cssModel, 'resolveValues').resolves([]); + const stylePropertyTreeElement = getTreeElement('left', '2%'); + stylePropertyTreeElement.updateTitle(); + + assert.isTrue(resolveValuesStub.calledOnce); + assert.strictEqual(resolveValuesStub.args[0][0], 'left'); + }); }); describe('MathFunctionRenderer', () => { @@ -1715,13 +1726,29 @@ describeWithMockConnection('StylePropertyTreeElement', () => { view.showTrace( property, null, matchedStyles, new Map(), Elements.StylePropertyTreeElement.getPropertyRenderers( - property.ownerStyle, stylesSidebarPane, matchedStyles, null, new Map())); + property.name, property.ownerStyle, stylesSidebarPane, matchedStyles, null, new Map())); assert.isTrue(evaluationSpy.calledOnce); const originalText = evaluationSpy.args[0][0].textContent; await evaluationSpy.returnValues[0]; assert.strictEqual(originalText, evaluationSpy.args[0][0].textContent); }); + + it('shows the original text during tracing when evaluation fails', async () => { + const cssModel = stylesSidebarPane.cssModel(); + assert.exists(cssModel); + const resolveValuesStub = sinon.stub(cssModel, 'resolveValues').resolves([]); + const property = addProperty('width', 'calc(1 + 1)'); + + const view = new Elements.CSSValueTraceView.CSSValueTraceView(undefined, () => {}); + view.showTrace( + property, null, matchedStyles, new Map(), + Elements.StylePropertyTreeElement.getPropertyRenderers( + property.name, property.ownerStyle, stylesSidebarPane, matchedStyles, null, new Map())); + + assert.isTrue(resolveValuesStub.calledOnce); + assert.strictEqual(resolveValuesStub.args[0][0], 'width'); + }); }); describe('AutoBaseRenderer', () => { diff --git a/front_end/panels/elements/StylePropertyTreeElement.ts b/front_end/panels/elements/StylePropertyTreeElement.ts index 45effd5857..616aa31d82 100644 --- a/front_end/panels/elements/StylePropertyTreeElement.ts +++ b/front_end/panels/elements/StylePropertyTreeElement.ts @@ -256,7 +256,7 @@ function getTracingTooltip( ?.getWidget() ?.showTrace( property, text, matchedStyles, computedStyles, - getPropertyRenderers( + getPropertyRenderers(property.name, property.ownerStyle, stylesPane, matchedStyles, null, computedStyles)); } @@ -308,7 +308,8 @@ export class VariableRenderer extends rendererBase(SDK.CSSPropertyParserMatchers const {nodes, cssControls} = Renderer.renderValueNodes( {name: declaration.name, value: declaration.value ?? ''}, substitution.cachedParsedValue(declaration.declaration, this.#matchedStyles, this.#computedStyles), - getPropertyRenderers(declaration.style, this.#stylesPane, this.#matchedStyles, null, this.#computedStyles), + getPropertyRenderers( + declaration.name, declaration.style, this.#stylesPane, this.#matchedStyles, null, this.#computedStyles), substitution); cssControls.forEach((value, key) => value.forEach(control => context.addControl(key, control))); return nodes; @@ -697,7 +698,7 @@ export class ColorMixRenderer extends rendererBase(SDK.CSSPropertyParserMatchers context.addControl('color', swatch); const nodeId = this.#pane.node()?.id; if (nodeId !== undefined) { - void this.#pane.cssModel()?.resolveValues(nodeId, colorMixText).then(results => { + void this.#pane.cssModel()?.resolveValues(undefined, nodeId, colorMixText).then(results => { if (results) { const color = Common.Color.parse(results[0]); if (color) { @@ -1291,11 +1292,13 @@ export class GridTemplateRenderer extends rendererBase(SDK.CSSPropertyParserMatc // clang-format off export class LengthRenderer extends rendererBase(SDK.CSSPropertyParserMatchers.LengthMatch) { - readonly #stylesPane: StylesSidebarPane; // clang-format on - constructor(stylesPane: StylesSidebarPane) { + readonly #stylesPane: StylesSidebarPane; + readonly #propertyName: string; + constructor(stylesPane: StylesSidebarPane, propertyName: string) { super(); this.#stylesPane = stylesPane; + this.#propertyName = propertyName; } override render(match: SDK.CSSPropertyParserMatchers.LengthMatch, context: RenderingContext): Node[] { @@ -1318,7 +1321,7 @@ export class LengthRenderer extends rendererBase(SDK.CSSPropertyParserMatchers.L return; } - const pixelValue = await this.#stylesPane.cssModel()?.resolveValues(nodeId, value); + const pixelValue = await this.#stylesPane.cssModel()?.resolveValues(this.#propertyName, nodeId, value); if (pixelValue) { valueElement.textContent = pixelValue[0]; @@ -1331,7 +1334,7 @@ export class LengthRenderer extends rendererBase(SDK.CSSPropertyParserMatchers.L return; } - const pixelValue = await this.#stylesPane.cssModel()?.resolveValues(nodeId, value); + const pixelValue = await this.#stylesPane.cssModel()?.resolveValues(this.#propertyName, nodeId, value); if (!pixelValue) { return; } @@ -1351,17 +1354,19 @@ export class LengthRenderer extends rendererBase(SDK.CSSPropertyParserMatchers.L // clang-format off export class MathFunctionRenderer extends rendererBase(SDK.CSSPropertyParserMatchers.MathFunctionMatch) { - readonly #stylesPane: StylesSidebarPane; - #matchedStyles: SDK.CSSMatchedStyles.CSSMatchedStyles; - #computedStyles: Map; // clang-format on + readonly #stylesPane: StylesSidebarPane; + #matchedStyles: SDK.CSSMatchedStyles.CSSMatchedStyles; + #computedStyles: Map; + #propertyName: string; constructor( stylesPane: StylesSidebarPane, matchedStyles: SDK.CSSMatchedStyles.CSSMatchedStyles, - computedStyles: Map) { + computedStyles: Map, propertyName: string) { super(); this.#matchedStyles = matchedStyles; this.#computedStyles = computedStyles; this.#stylesPane = stylesPane; + this.#propertyName = propertyName; } override render(match: SDK.CSSPropertyParserMatchers.MathFunctionMatch, context: RenderingContext): Node[] { @@ -1397,7 +1402,7 @@ export class MathFunctionRenderer extends rendererBase(SDK.CSSPropertyParserMatc if (nodeId === undefined) { return; } - const evaled = await this.#stylesPane.cssModel()?.resolveValues(nodeId, value); + const evaled = await this.#stylesPane.cssModel()?.resolveValues(this.#propertyName, nodeId, value); if (!evaled?.[0] || evaled[0] === value) { return; } @@ -1413,7 +1418,7 @@ export class MathFunctionRenderer extends rendererBase(SDK.CSSPropertyParserMatc // and compare the function result to the values of all its arguments. Evaluating the arguments eliminates nested // function calls and normalizes all units to px. values.unshift(functionText); - const evaledArgs = await this.#stylesPane.cssModel()?.resolveValues(nodeId, ...values); + const evaledArgs = await this.#stylesPane.cssModel()?.resolveValues(this.#propertyName, nodeId, ...values); if (!evaledArgs) { return; } @@ -1555,7 +1560,7 @@ export class PositionTryRenderer extends rendererBase(SDK.CSSPropertyParserMatch } export function getPropertyRenderers( - style: SDK.CSSStyleDeclaration.CSSStyleDeclaration, stylesPane: StylesSidebarPane, + propertyName: string, style: SDK.CSSStyleDeclaration.CSSStyleDeclaration, stylesPane: StylesSidebarPane, matchedStyles: SDK.CSSMatchedStyles.CSSMatchedStyles, treeElement: StylePropertyTreeElement|null, computedStyles: Map): Array> { return [ @@ -1576,8 +1581,8 @@ export function getPropertyRenderers( new PositionAnchorRenderer(stylesPane), new FlexGridRenderer(stylesPane, treeElement), new PositionTryRenderer(matchedStyles), - new LengthRenderer(stylesPane), - new MathFunctionRenderer(stylesPane, matchedStyles, computedStyles), + new LengthRenderer(stylesPane, propertyName), + new MathFunctionRenderer(stylesPane, matchedStyles, computedStyles, propertyName), new AutoBaseRenderer(computedStyles), new BinOpRenderer(), ]; @@ -1999,10 +2004,11 @@ export class StylePropertyTreeElement extends UI.TreeOutline.TreeElement { this.expandElement.setAttribute('jslog', `${VisualLogging.expand().track({click: true})}`); } - const renderers = this.property.parsedOk ? getPropertyRenderers( - this.style, this.parentPaneInternal, this.matchedStylesInternal, - this, this.getComputedStyles() ?? new Map()) : - []; + const renderers = this.property.parsedOk ? + getPropertyRenderers( + this.name, this.style, this.parentPaneInternal, this.matchedStylesInternal, this, + this.getComputedStyles() ?? new Map()) : + []; if (Root.Runtime.experiments.isEnabled('font-editor') && this.property.parsedOk) { renderers.push(new FontRenderer(this));