Skip to content

Commit a792c7e

Browse files
pfaffeDevtools-frontend LUCI CQ
authored andcommitted
Revert "[css value tracing] evaluate percentages in longhands"
This reverts commit a0c4c4a. Reason for revert: The change breaks a bunch of non-percentage evaluations. Original change's description: > [css value tracing] 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: I823ef9d52bb12ab40e7bcabce770733ad36a51b7 > Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6376104 > Reviewed-by: Eric Leese <[email protected]> > Auto-Submit: Philip Pfaffe <[email protected]> > Commit-Queue: Eric Leese <[email protected]> Bug: 401213719 Change-Id: I19d3567542f9b53e57efa9747f70d3aa529acec2 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6405417 Auto-Submit: Philip Pfaffe <[email protected]> Commit-Queue: Rubber Stamper <[email protected]> Bot-Commit: Rubber Stamper <[email protected]>
1 parent d3465fc commit a792c7e

File tree

5 files changed

+31
-72
lines changed

5 files changed

+31
-72
lines changed

front_end/core/sdk/CSSModel.ts

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ 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';
4746
import {CSSStyleRule} from './CSSRule.js';
4847
import {CSSStyleDeclaration, Type} from './CSSStyleDeclaration.js';
4948
import {CSSStyleSheetHeader} from './CSSStyleSheetHeader.js';
@@ -118,16 +117,9 @@ export class CSSModel extends SDKModel<EventTypes> {
118117
return this.#colorScheme;
119118
}
120119

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;
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;
131123
}
132124

133125
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
@@ -684,10 +684,10 @@ export class LengthMatch implements Match {
684684
export class LengthMatcher extends matcherBase(LengthMatch) {
685685
// clang-format on
686686
static readonly LENGTH_UNITS = new Set([
687-
'em', 'ex', 'ch', 'cap', 'ic', 'lh', 'rem', 'rex', 'rch', 'rlh', 'ric', 'rcap', 'pt', 'pc',
688-
'in', 'cm', 'mm', 'Q', 'vw', 'vh', 'vi', 'vb', 'vmin', 'vmax', 'dvw', 'dvh', 'dvi', 'dvb',
689-
'dvmin', 'dvmax', 'svw', 'svh', 'svi', 'svb', 'svmin', 'svmax', 'lvw', 'lvh', 'lvi', 'lvb', 'lvmin', 'lvmax',
690-
'cqw', 'cqh', 'cqi', 'cqb', 'cqmin', 'cqmax', 'cqem', 'cqlh', 'cqex', 'cqch', '%'
687+
'em', 'ex', 'ch', 'cap', 'ic', 'lh', 'rem', 'rex', 'rch', 'rlh', 'ric', 'rcap', 'pt',
688+
'pc', 'in', 'cm', 'mm', 'Q', 'vw', 'vh', 'vi', 'vb', 'vmin', 'vmax', 'dvw', 'dvh',
689+
'dvi', 'dvb', 'dvmin', 'dvmax', 'svw', 'svh', 'svi', 'svb', 'svmin', 'svmax', 'lvw', 'lvh', 'lvi',
690+
'lvb', 'lvmin', 'lvmax', 'cqw', 'cqh', 'cqi', 'cqb', 'cqmin', 'cqmax', 'cqem', 'cqlh', 'cqex', 'cqch',
691691
]);
692692
override matches(node: CodeMirror.SyntaxNode, matching: BottomUpTreeMatching): LengthMatch|null {
693693
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.name, property.ownerStyle, treeElement.parentPane(), matchedStyles, treeElement,
76+
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: 3 additions & 30 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-
property.name, matchedStyles.nodeStyles()[0], stylesSidebarPane, matchedStyles, null, new Map()),
289+
matchedStyles.nodeStyles()[0], stylesSidebarPane, matchedStyles, null, new Map()),
290290
context);
291291

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

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

16801669
describe('MathFunctionRenderer', () => {
@@ -1726,29 +1715,13 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
17261715
view.showTrace(
17271716
property, null, matchedStyles, new Map(),
17281717
Elements.StylePropertyTreeElement.getPropertyRenderers(
1729-
property.name, property.ownerStyle, stylesSidebarPane, matchedStyles, null, new Map()));
1718+
property.ownerStyle, stylesSidebarPane, matchedStyles, null, new Map()));
17301719

17311720
assert.isTrue(evaluationSpy.calledOnce);
17321721
const originalText = evaluationSpy.args[0][0].textContent;
17331722
await evaluationSpy.returnValues[0];
17341723
assert.strictEqual(originalText, evaluationSpy.args[0][0].textContent);
17351724
});
1736-
1737-
it('shows the original text during tracing when evaluation fails', async () => {
1738-
const cssModel = stylesSidebarPane.cssModel();
1739-
assert.exists(cssModel);
1740-
const resolveValuesStub = sinon.stub(cssModel, 'resolveValues').resolves([]);
1741-
const property = addProperty('width', 'calc(1 + 1)');
1742-
1743-
const view = new Elements.CSSValueTraceView.CSSValueTraceView(undefined, () => {});
1744-
view.showTrace(
1745-
property, null, matchedStyles, new Map(),
1746-
Elements.StylePropertyTreeElement.getPropertyRenderers(
1747-
property.name, property.ownerStyle, stylesSidebarPane, matchedStyles, null, new Map()));
1748-
1749-
assert.isTrue(resolveValuesStub.calledOnce);
1750-
assert.strictEqual(resolveValuesStub.args[0][0], 'width');
1751-
});
17521725
});
17531726

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

front_end/panels/elements/StylePropertyTreeElement.ts

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ function getTracingTooltip(
256256
?.getWidget()
257257
?.showTrace(
258258
property, text, matchedStyles, computedStyles,
259-
getPropertyRenderers(property.name,
259+
getPropertyRenderers(
260260
property.ownerStyle, stylesPane, matchedStyles, null,
261261
computedStyles));
262262
}
@@ -308,8 +308,7 @@ export class VariableRenderer extends rendererBase(SDK.CSSPropertyParserMatchers
308308
const {nodes, cssControls} = Renderer.renderValueNodes(
309309
{name: declaration.name, value: declaration.value ?? ''},
310310
substitution.cachedParsedValue(declaration.declaration, this.#matchedStyles, this.#computedStyles),
311-
getPropertyRenderers(
312-
declaration.name, declaration.style, this.#stylesPane, this.#matchedStyles, null, this.#computedStyles),
311+
getPropertyRenderers(declaration.style, this.#stylesPane, this.#matchedStyles, null, this.#computedStyles),
313312
substitution);
314313
cssControls.forEach((value, key) => value.forEach(control => context.addControl(key, control)));
315314
return nodes;
@@ -698,7 +697,7 @@ export class ColorMixRenderer extends rendererBase(SDK.CSSPropertyParserMatchers
698697
context.addControl('color', swatch);
699698
const nodeId = this.#pane.node()?.id;
700699
if (nodeId !== undefined) {
701-
void this.#pane.cssModel()?.resolveValues(undefined, nodeId, colorMixText).then(results => {
700+
void this.#pane.cssModel()?.resolveValues(nodeId, colorMixText).then(results => {
702701
if (results) {
703702
const color = Common.Color.parse(results[0]);
704703
if (color) {
@@ -1292,13 +1291,11 @@ export class GridTemplateRenderer extends rendererBase(SDK.CSSPropertyParserMatc
12921291

12931292
// clang-format off
12941293
export class LengthRenderer extends rendererBase(SDK.CSSPropertyParserMatchers.LengthMatch) {
1295-
// clang-format on
12961294
readonly #stylesPane: StylesSidebarPane;
1297-
readonly #propertyName: string;
1298-
constructor(stylesPane: StylesSidebarPane, propertyName: string) {
1295+
// clang-format on
1296+
constructor(stylesPane: StylesSidebarPane) {
12991297
super();
13001298
this.#stylesPane = stylesPane;
1301-
this.#propertyName = propertyName;
13021299
}
13031300

13041301
override render(match: SDK.CSSPropertyParserMatchers.LengthMatch, context: RenderingContext): Node[] {
@@ -1321,7 +1318,7 @@ export class LengthRenderer extends rendererBase(SDK.CSSPropertyParserMatchers.L
13211318
return;
13221319
}
13231320

1324-
const pixelValue = await this.#stylesPane.cssModel()?.resolveValues(this.#propertyName, nodeId, value);
1321+
const pixelValue = await this.#stylesPane.cssModel()?.resolveValues(nodeId, value);
13251322

13261323
if (pixelValue) {
13271324
valueElement.textContent = pixelValue[0];
@@ -1334,7 +1331,7 @@ export class LengthRenderer extends rendererBase(SDK.CSSPropertyParserMatchers.L
13341331
return;
13351332
}
13361333

1337-
const pixelValue = await this.#stylesPane.cssModel()?.resolveValues(this.#propertyName, nodeId, value);
1334+
const pixelValue = await this.#stylesPane.cssModel()?.resolveValues(nodeId, value);
13381335
if (!pixelValue) {
13391336
return;
13401337
}
@@ -1354,19 +1351,17 @@ export class LengthRenderer extends rendererBase(SDK.CSSPropertyParserMatchers.L
13541351

13551352
// clang-format off
13561353
export class MathFunctionRenderer extends rendererBase(SDK.CSSPropertyParserMatchers.MathFunctionMatch) {
1357-
// clang-format on
13581354
readonly #stylesPane: StylesSidebarPane;
1359-
#matchedStyles: SDK.CSSMatchedStyles.CSSMatchedStyles;
1360-
#computedStyles: Map<string, string>;
1361-
#propertyName: string;
1355+
#matchedStyles: SDK.CSSMatchedStyles.CSSMatchedStyles;
1356+
#computedStyles: Map<string, string>;
1357+
// clang-format on
13621358
constructor(
13631359
stylesPane: StylesSidebarPane, matchedStyles: SDK.CSSMatchedStyles.CSSMatchedStyles,
1364-
computedStyles: Map<string, string>, propertyName: string) {
1360+
computedStyles: Map<string, string>) {
13651361
super();
13661362
this.#matchedStyles = matchedStyles;
13671363
this.#computedStyles = computedStyles;
13681364
this.#stylesPane = stylesPane;
1369-
this.#propertyName = propertyName;
13701365
}
13711366

13721367
override render(match: SDK.CSSPropertyParserMatchers.MathFunctionMatch, context: RenderingContext): Node[] {
@@ -1402,7 +1397,7 @@ export class MathFunctionRenderer extends rendererBase(SDK.CSSPropertyParserMatc
14021397
if (nodeId === undefined) {
14031398
return;
14041399
}
1405-
const evaled = await this.#stylesPane.cssModel()?.resolveValues(this.#propertyName, nodeId, value);
1400+
const evaled = await this.#stylesPane.cssModel()?.resolveValues(nodeId, value);
14061401
if (!evaled?.[0] || evaled[0] === value) {
14071402
return;
14081403
}
@@ -1418,7 +1413,7 @@ export class MathFunctionRenderer extends rendererBase(SDK.CSSPropertyParserMatc
14181413
// and compare the function result to the values of all its arguments. Evaluating the arguments eliminates nested
14191414
// function calls and normalizes all units to px.
14201415
values.unshift(functionText);
1421-
const evaledArgs = await this.#stylesPane.cssModel()?.resolveValues(this.#propertyName, nodeId, ...values);
1416+
const evaledArgs = await this.#stylesPane.cssModel()?.resolveValues(nodeId, ...values);
14221417
if (!evaledArgs) {
14231418
return;
14241419
}
@@ -1560,7 +1555,7 @@ export class PositionTryRenderer extends rendererBase(SDK.CSSPropertyParserMatch
15601555
}
15611556

15621557
export function getPropertyRenderers(
1563-
propertyName: string, style: SDK.CSSStyleDeclaration.CSSStyleDeclaration, stylesPane: StylesSidebarPane,
1558+
style: SDK.CSSStyleDeclaration.CSSStyleDeclaration, stylesPane: StylesSidebarPane,
15641559
matchedStyles: SDK.CSSMatchedStyles.CSSMatchedStyles, treeElement: StylePropertyTreeElement|null,
15651560
computedStyles: Map<string, string>): Array<MatchRenderer<SDK.CSSPropertyParser.Match>> {
15661561
return [
@@ -1581,8 +1576,8 @@ export function getPropertyRenderers(
15811576
new PositionAnchorRenderer(stylesPane),
15821577
new FlexGridRenderer(stylesPane, treeElement),
15831578
new PositionTryRenderer(matchedStyles),
1584-
new LengthRenderer(stylesPane, propertyName),
1585-
new MathFunctionRenderer(stylesPane, matchedStyles, computedStyles, propertyName),
1579+
new LengthRenderer(stylesPane),
1580+
new MathFunctionRenderer(stylesPane, matchedStyles, computedStyles),
15861581
new AutoBaseRenderer(computedStyles),
15871582
new BinOpRenderer(),
15881583
];
@@ -1981,11 +1976,10 @@ export class StylePropertyTreeElement extends UI.TreeOutline.TreeElement {
19811976
this.expandElement.setAttribute('jslog', `${VisualLogging.expand().track({click: true})}`);
19821977
}
19831978

1984-
const renderers = this.property.parsedOk ?
1985-
getPropertyRenderers(
1986-
this.name, this.style, this.parentPaneInternal, this.matchedStylesInternal, this,
1987-
this.getComputedStyles() ?? new Map()) :
1988-
[];
1979+
const renderers = this.property.parsedOk ? getPropertyRenderers(
1980+
this.style, this.parentPaneInternal, this.matchedStylesInternal,
1981+
this, this.getComputedStyles() ?? new Map()) :
1982+
[];
19891983

19901984
if (Root.Runtime.experiments.isEnabled('font-editor') && this.property.parsedOk) {
19911985
renderers.push(new FontRenderer(this));

0 commit comments

Comments
 (0)