Nothing Special   »   [go: up one dir, main page]

Closed Bug 1866712 Opened 11 months ago Closed 5 months ago

Mark custom property declaration invalid when value does not match registered custom property definition

Categories

(DevTools :: Inspector: Rules, task)

task

Tracking

(firefox128 fixed)

RESOLVED FIXED
128 Branch
Tracking Status
firefox128 --- fixed

People

(Reporter: nchevobbe, Assigned: nchevobbe)

References

(Blocks 2 open bugs)

Details

Attachments

(3 files)

Steps to reproduce

  1. Navigate to data:text/html,<meta charset=utf8><style>@property --b { syntax: '<length>'; inherits: true; initial-value: 100px; } h1 { --b: blue;color: var(--b); }</style><h1>hello</h1>
  2. Open the inspector and select the <h1> node

Expected results

The --b: blue property is marked as invalid, as it doesn't match the <length> syntax.
(color: var(--b) property should also be marked as invalid but that may be its own bug since we don't do it for unregistered custom properties at the moment)


In Chrome DevTools, a tooltip is displayed to indicate why the property value is invalid, which is a good information to surface IMO

Component: General → Inspector: Rules
Attachment #9365530 - Attachment description: CleanShot 2023-11-27 at 08.45.39.png → popup on custom property definition, indicating that syntax doesn't match the expected one
Attachment #9365530 - Attachment filename: CleanShot-2023-11-27-at-08.45.39.png → Chrome DevTools Screenshot
Attachment #9365530 - Attachment description: popup on custom property definition, indicating that syntax doesn't match the expected one → Chrome DevTools Screenshot

This impacts the var() tooltip: if the value doesn't match the syntax, the registered property initial value will be used.
This also impacts the computed panel: when "expanding" the variable node, the declaration that we show should be marked as invalid/unused somehow. We could also show the initial value declaration, but that's maybe its own bug

Multiple functions were writing the same 2 lines to retrieve the associated
declaration (and I'm planning to add more).
Let's refactor things a bit.

Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED

This adds an icon next to custom property declaration for registered property
when a value does not match the registered property syntax (e.g. it's invalid
at computed-value time).
The icon title indicates the syntax that is expected.

Attachment #9405017 - Attachment description: WIP: Bug 1866712 - [devtools] Display invalid at computed value time custom property declarations. #devtools-reviewers. → Bug 1866712 - [devtools] Display invalid at computed value time custom property declarations. #devtools-reviewers.

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #1)

This impacts the var() tooltip: if the value doesn't match the syntax, the registered property initial value will be used.

This isn't always true, the value is substituted with unset, which means the inherited value could be used. We'll handle this in Bug 1900064.

This also impacts the computed panel: when "expanding" the variable node, the declaration that we show should be marked as invalid/unused somehow. We could also show the initial value declaration, but that's maybe its own bug

This should be handled in Bug 1900070

Attachment #9405017 - Attachment description: Bug 1866712 - [devtools] Display invalid at computed value time custom property declarations. #devtools-reviewers. → WIP: Bug 1866712 - [devtools] Display invalid at computed value time custom property declarations. #devtools-reviewers.
Attachment #9405017 - Attachment description: WIP: Bug 1866712 - [devtools] Display invalid at computed value time custom property declarations. #devtools-reviewers. → Bug 1866712 - [devtools] Display invalid at computed value time custom property declarations. #devtools-reviewers.
Attachment #9405017 - Attachment description: Bug 1866712 - [devtools] Display invalid at computed value time custom property declarations. #devtools-reviewers. → WIP: Bug 1866712 - [devtools] Display invalid at computed value time custom property declarations. #devtools-reviewers.
Attachment #9405017 - Attachment description: WIP: Bug 1866712 - [devtools] Display invalid at computed value time custom property declarations. #devtools-reviewers. → Bug 1866712 - [devtools] Display invalid at computed value time custom property declarations. #devtools-reviewers.
Attachment #9405017 - Attachment description: Bug 1866712 - [devtools] Display invalid at computed value time custom property declarations. #devtools-reviewers. → WIP: Bug 1866712 - [devtools] Display invalid at computed value time custom property declarations. #devtools-reviewers.
Attachment #9405017 - Attachment description: WIP: Bug 1866712 - [devtools] Display invalid at computed value time custom property declarations. #devtools-reviewers. → Bug 1866712 - [devtools] Display invalid at computed value time custom property declarations. r=#devtools-reviewers.
Pushed by nchevobbe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/99a7a139957d [devtools] Add TextProperty#getDomRuleDeclaration. r=devtools-reviewers,bomsy. https://hg.mozilla.org/integration/autoland/rev/afababa6dd0e [devtools] Display invalid at computed value time custom property declarations. r=devtools-reviewers,bomsy.
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 128 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: