Skip to content

Commit 3fb8c84

Browse files
pfaffeDevtools-frontend LUCI CQ
authored andcommitted
Revert "[css value tracing] Reland: evaluate percentages in longhands"
This reverts commit a853f7e. Reason for revert: This still creates lots of weirdness in non-length properties. Original change's description: > [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]> Bug: 401213719 Change-Id: I1c214ecc429371bf6dbc934447bdf4e01ac6579b Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6439640 Auto-Submit: Philip Pfaffe <[email protected]> Commit-Queue: Rubber Stamper <[email protected]> Reviewed-by: Eric Leese <[email protected]> Bot-Commit: Rubber Stamper <[email protected]>
1 parent adb1cd0 commit 3fb8c84

File tree

5 files changed

+27
-68
lines changed

5 files changed

+27
-68
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
@@ -683,10 +683,10 @@ export class LengthMatch implements Match {
683683
export class LengthMatcher extends matcherBase(LengthMatch) {
684684
// clang-format on
685685
static readonly LENGTH_UNITS = new Set([
686-
'em', 'ex', 'ch', 'cap', 'ic', 'lh', 'rem', 'rex', 'rch', 'rlh', 'ric', 'rcap', 'pt', 'pc',
687-
'in', 'cm', 'mm', 'Q', 'vw', 'vh', 'vi', 'vb', 'vmin', 'vmax', 'dvw', 'dvh', 'dvi', 'dvb',
688-
'dvmin', 'dvmax', 'svw', 'svh', 'svi', 'svb', 'svmin', 'svmax', 'lvw', 'lvh', 'lvi', 'lvb', 'lvmin', 'lvmax',
689-
'cqw', 'cqh', 'cqi', 'cqb', 'cqmin', 'cqmax', 'cqem', 'cqlh', 'cqex', 'cqch', '%'
686+
'em', 'ex', 'ch', 'cap', 'ic', 'lh', 'rem', 'rex', 'rch', 'rlh', 'ric', 'rcap', 'pt',
687+
'pc', 'in', 'cm', 'mm', 'Q', 'vw', 'vh', 'vi', 'vb', 'vmin', 'vmax', 'dvw', 'dvh',
688+
'dvi', 'dvb', 'dvmin', 'dvmax', 'svw', 'svh', 'svi', 'svb', 'svmin', 'svmax', 'lvw', 'lvh', 'lvi',
689+
'lvb', 'lvmin', 'lvmax', 'cqw', 'cqh', 'cqi', 'cqb', 'cqmin', 'cqmax', 'cqem', 'cqlh', 'cqex', 'cqch',
690690
]);
691691
override matches(node: CodeMirror.SyntaxNode, matching: BottomUpTreeMatching): LengthMatch|null {
692692
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');
@@ -1667,7 +1667,7 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
16671667
const addPopoverPromise = Promise.withResolvers<void>();
16681668
sinon.stub(Elements.StylePropertyTreeElement.LengthRenderer.prototype, 'popOverAttachedForTest')
16691669
.callsFake(() => addPopoverPromise.resolve());
1670-
const stylePropertyTreeElement = getTreeElement('property', '5px 2em');
1670+
const stylePropertyTreeElement = getTreeElement('margin', '5px 2em');
16711671
setMockConnectionResponseHandler('CSS.getComputedStyleForNode', () => ({computedStyle: {}}));
16721672

16731673
await stylePropertyTreeElement.onpopulate();
@@ -1676,17 +1676,6 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
16761676
const popover = stylePropertyTreeElement.valueElement?.querySelector('devtools-tooltip');
16771677
assert.strictEqual(popover?.innerText, '15px');
16781678
});
1679-
1680-
it('passes the property name to evaluations', async () => {
1681-
const cssModel = stylesSidebarPane.cssModel();
1682-
assert.exists(cssModel);
1683-
const resolveValuesStub = sinon.stub(cssModel, 'resolveValues').resolves([]);
1684-
const stylePropertyTreeElement = getTreeElement('left', '2%');
1685-
stylePropertyTreeElement.updateTitle();
1686-
1687-
assert.isTrue(resolveValuesStub.calledOnce);
1688-
assert.strictEqual(resolveValuesStub.args[0][0], 'left');
1689-
});
16901679
});
16911680

16921681
describe('MathFunctionRenderer', () => {
@@ -1739,29 +1728,13 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
17391728
view.showTrace(
17401729
property, null, matchedStyles, new Map(),
17411730
Elements.StylePropertyTreeElement.getPropertyRenderers(
1742-
property.name, property.ownerStyle, stylesSidebarPane, matchedStyles, null, new Map()));
1731+
property.ownerStyle, stylesSidebarPane, matchedStyles, null, new Map()));
17431732

17441733
assert.isTrue(evaluationSpy.calledOnce);
17451734
const originalText = evaluationSpy.args[0][0].textContent;
17461735
await evaluationSpy.returnValues[0];
17471736
assert.strictEqual(originalText, evaluationSpy.args[0][0].textContent);
17481737
});
1749-
1750-
it('shows the original text during tracing when evaluation fails', async () => {
1751-
const cssModel = stylesSidebarPane.cssModel();
1752-
assert.exists(cssModel);
1753-
const resolveValuesStub = sinon.stub(cssModel, 'resolveValues').resolves([]);
1754-
const property = addProperty('width', 'calc(1 + 1)');
1755-
1756-
const view = new Elements.CSSValueTraceView.CSSValueTraceView(undefined, () => {});
1757-
view.showTrace(
1758-
property, null, matchedStyles, new Map(),
1759-
Elements.StylePropertyTreeElement.getPropertyRenderers(
1760-
property.name, property.ownerStyle, stylesSidebarPane, matchedStyles, null, new Map()));
1761-
1762-
assert.isTrue(resolveValuesStub.calledOnce);
1763-
assert.strictEqual(resolveValuesStub.args[0][0], 'width');
1764-
});
17651738
});
17661739

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

front_end/panels/elements/StylePropertyTreeElement.ts

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,7 @@ 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(
266-
declaration.name, declaration.style, this.#stylesPane, this.#matchedStyles, null, this.#computedStyles),
265+
getPropertyRenderers(declaration.style, this.#stylesPane, this.#matchedStyles, null, this.#computedStyles),
267266
substitution);
268267
cssControls.forEach((value, key) => value.forEach(control => context.addControl(key, control)));
269268
return nodes;
@@ -662,7 +661,7 @@ export class ColorMixRenderer extends rendererBase(SDK.CSSPropertyParserMatchers
662661
context.addControl('color', swatch);
663662
const nodeId = this.#pane.node()?.id;
664663
if (nodeId !== undefined) {
665-
void this.#pane.cssModel()?.resolveValues(undefined, nodeId, colorMixText).then(results => {
664+
void this.#pane.cssModel()?.resolveValues(nodeId, colorMixText).then(results => {
666665
if (results) {
667666
const color = Common.Color.parse(results[0]);
668667
if (color) {
@@ -1262,12 +1261,10 @@ export class LengthRenderer extends rendererBase(SDK.CSSPropertyParserMatchers.L
12621261
// clang-format on
12631262
readonly #stylesPane: StylesSidebarPane;
12641263
readonly #treeElement: StylePropertyTreeElement|null;
1265-
readonly #propertyName: string;
1266-
constructor(stylesPane: StylesSidebarPane, propertyName: string, treeElement: StylePropertyTreeElement|null) {
1264+
constructor(stylesPane: StylesSidebarPane, treeElement: StylePropertyTreeElement|null) {
12671265
super();
12681266
this.#stylesPane = stylesPane;
12691267
this.#treeElement = treeElement;
1270-
this.#propertyName = propertyName;
12711268
}
12721269

12731270
override render(match: SDK.CSSPropertyParserMatchers.LengthMatch, context: RenderingContext): Node[] {
@@ -1290,7 +1287,7 @@ export class LengthRenderer extends rendererBase(SDK.CSSPropertyParserMatchers.L
12901287
return;
12911288
}
12921289

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

12951292
if (pixelValue) {
12961293
valueElement.textContent = pixelValue[0];
@@ -1303,7 +1300,7 @@ export class LengthRenderer extends rendererBase(SDK.CSSPropertyParserMatchers.L
13031300
return;
13041301
}
13051302

1306-
const pixelValue = await this.#stylesPane.cssModel()?.resolveValues(this.#propertyName, nodeId, value);
1303+
const pixelValue = await this.#stylesPane.cssModel()?.resolveValues(nodeId, value);
13071304
if (!pixelValue) {
13081305
return;
13091306
}
@@ -1330,16 +1327,14 @@ export class MathFunctionRenderer extends rendererBase(SDK.CSSPropertyParserMatc
13301327
readonly #matchedStyles: SDK.CSSMatchedStyles.CSSMatchedStyles;
13311328
readonly #computedStyles: Map<string, string>;
13321329
readonly #treeElement: StylePropertyTreeElement|null;
1333-
readonly #propertyName: string;
13341330
constructor(
13351331
stylesPane: StylesSidebarPane, matchedStyles: SDK.CSSMatchedStyles.CSSMatchedStyles,
1336-
computedStyles: Map<string, string>, propertyName: string, treeElement: StylePropertyTreeElement|null) {
1332+
computedStyles: Map<string, string>, treeElement: StylePropertyTreeElement|null) {
13371333
super();
13381334
this.#matchedStyles = matchedStyles;
13391335
this.#computedStyles = computedStyles;
13401336
this.#stylesPane = stylesPane;
13411337
this.#treeElement = treeElement;
1342-
this.#propertyName = propertyName;
13431338
}
13441339

13451340
override render(match: SDK.CSSPropertyParserMatchers.MathFunctionMatch, context: RenderingContext): Node[] {
@@ -1375,7 +1370,7 @@ export class MathFunctionRenderer extends rendererBase(SDK.CSSPropertyParserMatc
13751370
if (nodeId === undefined) {
13761371
return;
13771372
}
1378-
const evaled = await this.#stylesPane.cssModel()?.resolveValues(this.#propertyName, nodeId, value);
1373+
const evaled = await this.#stylesPane.cssModel()?.resolveValues(nodeId, value);
13791374
if (!evaled?.[0] || evaled[0] === value) {
13801375
return;
13811376
}
@@ -1391,7 +1386,7 @@ export class MathFunctionRenderer extends rendererBase(SDK.CSSPropertyParserMatc
13911386
// and compare the function result to the values of all its arguments. Evaluating the arguments eliminates nested
13921387
// function calls and normalizes all units to px.
13931388
values.unshift(functionText);
1394-
const evaledArgs = await this.#stylesPane.cssModel()?.resolveValues(this.#propertyName, nodeId, ...values);
1389+
const evaledArgs = await this.#stylesPane.cssModel()?.resolveValues(nodeId, ...values);
13951390
if (!evaledArgs) {
13961391
return;
13971392
}
@@ -1533,7 +1528,7 @@ export class PositionTryRenderer extends rendererBase(SDK.CSSPropertyParserMatch
15331528
}
15341529

15351530
export function getPropertyRenderers(
1536-
propertyName: string, style: SDK.CSSStyleDeclaration.CSSStyleDeclaration, stylesPane: StylesSidebarPane,
1531+
style: SDK.CSSStyleDeclaration.CSSStyleDeclaration, stylesPane: StylesSidebarPane,
15371532
matchedStyles: SDK.CSSMatchedStyles.CSSMatchedStyles, treeElement: StylePropertyTreeElement|null,
15381533
computedStyles: Map<string, string>): Array<MatchRenderer<SDK.CSSPropertyParser.Match>> {
15391534
return [
@@ -1554,8 +1549,8 @@ export function getPropertyRenderers(
15541549
new PositionAnchorRenderer(stylesPane),
15551550
new FlexGridRenderer(stylesPane, treeElement),
15561551
new PositionTryRenderer(matchedStyles),
1557-
new LengthRenderer(stylesPane, propertyName, treeElement),
1558-
new MathFunctionRenderer(stylesPane, matchedStyles, computedStyles, propertyName, treeElement),
1552+
new LengthRenderer(stylesPane, treeElement),
1553+
new MathFunctionRenderer(stylesPane, matchedStyles, computedStyles, treeElement),
15591554
new AutoBaseRenderer(computedStyles),
15601555
new BinOpRenderer(),
15611556
];
@@ -1959,11 +1954,10 @@ export class StylePropertyTreeElement extends UI.TreeOutline.TreeElement {
19591954
this.expandElement.setAttribute('jslog', `${VisualLogging.expand().track({click: true})}`);
19601955
}
19611956

1962-
const renderers = this.property.parsedOk ?
1963-
getPropertyRenderers(
1964-
this.name, this.style, this.parentPaneInternal, this.matchedStylesInternal, this,
1965-
this.getComputedStyles() ?? new Map()) :
1966-
[];
1957+
const renderers = this.property.parsedOk ? getPropertyRenderers(
1958+
this.style, this.parentPaneInternal, this.matchedStylesInternal,
1959+
this, this.getComputedStyles() ?? new Map()) :
1960+
[];
19671961

19681962
if (Root.Runtime.experiments.isEnabled('font-editor') && this.property.parsedOk) {
19691963
renderers.push(new FontRenderer(this));
@@ -2127,7 +2121,7 @@ export class StylePropertyTreeElement extends UI.TreeOutline.TreeElement {
21272121
?.getWidget()
21282122
?.showTrace(
21292123
property, text, matchedStyles, computedStyles,
2130-
getPropertyRenderers(property.name,
2124+
getPropertyRenderers(
21312125
property.ownerStyle, stylesPane, matchedStyles, null, computedStyles));
21322126
}
21332127
}}

0 commit comments

Comments
 (0)