Skip to content

Commit 3dd344a

Browse files
hanselfmu-chromiumDevtools-frontend LUCI CQ
authored andcommitted
Add squiggly line support for <select> issues in DOM tree
This CL adds a separate utility collection for issues in the Elements panel, which will be used for both the DOM tree and the a11y tree. This utility collection is responsible for adapting various types of issues and return "normalized" result for the Elements panel to display. This way, we keep the DOM tree agnostic to the adaption and the types of issues, hence the relaxation of the issue type from GenericIssue to Issue. For now, the state management of Elements panel-related issues stays in the Elements outline. Bug: 378738916, 393245109 Change-Id: Ifbabc14b735cc0f7568e7a46bd7ea1807b52dc25 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6297955 Commit-Queue: Changhao Han <changhaohan@chromium.org> Reviewed-by: Philip Pfaffe <pfaffe@chromium.org>
1 parent 7943cd3 commit 3dd344a

8 files changed

Lines changed: 280 additions & 72 deletions

File tree

‎config/gni/devtools_grd_files.gni‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1395,6 +1395,7 @@ grd_files_debug_sources = [
13951395
"front_end/panels/elements/ComputedStyleWidget.js",
13961396
"front_end/panels/elements/DOMLinkifier.js",
13971397
"front_end/panels/elements/DOMPath.js",
1398+
"front_end/panels/elements/ElementIssueUtils.js",
13981399
"front_end/panels/elements/ElementStatePaneWidget.js",
13991400
"front_end/panels/elements/ElementsPanel.js",
14001401
"front_end/panels/elements/ElementsSidebarPane.js",

‎front_end/core/root/Runtime.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ export class ExperimentsSupport {
140140
experimentName: string, experimentTitle: string, unstable?: boolean, docLink?: string,
141141
feedbackLink?: string): void {
142142
if (this.#experimentNames.has(experimentName)) {
143-
throw new Error(`Duplicate registraction of experiment '${experimentName}'`);
143+
throw new Error(`Duplicate registration of experiment '${experimentName}'`);
144144
}
145145
this.#experimentNames.add(experimentName);
146146
this.#experiments.push(new Experiment(

‎front_end/entrypoints/main/MainImpl.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,6 @@ export class MainImpl {
338338
Root.Runtime.ExperimentName.STYLES_PANE_CSS_CHANGES, 'Sync CSS changes in the Styles tab');
339339

340340
// Highlights a violating node or attribute by rendering a squiggly line under it and adding a tooltip linking to the issues panel.
341-
// Right now violating nodes are exclusively form fields that contain an HTML issue, for example, and <input /> whose id is duplicate inside the form.
342341
Root.Runtime.experiments.register(
343342
Root.Runtime.ExperimentName.HIGHLIGHT_ERRORS_ELEMENTS_PANEL,
344343
'Highlights a violating node or attribute in the Elements panel DOM tree');

‎front_end/panels/elements/BUILD.gn‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ devtools_module("elements") {
4141
"ComputedStyleWidget.ts",
4242
"DOMLinkifier.ts",
4343
"DOMPath.ts",
44+
"ElementIssueUtils.ts",
4445
"ElementStatePaneWidget.ts",
4546
"ElementsPanel.ts",
4647
"ElementsSidebarPane.ts",
Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
// Copyright 2025 The Chromium Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import * as i18n from '../../core/i18n/i18n.js';
6+
import * as Protocol from '../../generated/protocol.js';
7+
import * as IssuesManager from '../../models/issues_manager/issues_manager.js';
8+
9+
const UIStrings = {
10+
/**
11+
* @description Tooltip text shown in the Elements panel when an element has an error.
12+
*/
13+
formLabelForNameError: 'Incorrect use of <label for=FORM_ELEMENT>',
14+
/**
15+
* @description Tooltip text shown in the Elements panel when an element has an error.
16+
*/
17+
formDuplicateIdForInputError: 'Duplicate form field id in the same form',
18+
/**
19+
* @description Tooltip text shown in the Elements panel when an element has an error.
20+
*/
21+
formInputWithNoLabelError: 'Form field without valid aria-labelledby attribute or associated label',
22+
/**
23+
* @description Tooltip text shown in the Elements panel when an element has an error.
24+
*/
25+
formAutocompleteAttributeEmptyError: 'Incorrect use of autocomplete attribute',
26+
/**
27+
* @description Tooltip text shown in the Elements panel when an element has an error.
28+
*/
29+
formEmptyIdAndNameAttributesForInputError: 'A form field element should have an id or name attribute',
30+
/**
31+
* @description Tooltip text shown in the Elements panel when an element has an error.
32+
*/
33+
formAriaLabelledByToNonExistingId: 'An aria-labelledby attribute doesn\'t match any element id',
34+
/**
35+
* @description Tooltip text shown in the Elements panel when an element has an error.
36+
*/
37+
formInputAssignedAutocompleteValueToIdOrNameAttributeError: 'An element doesn\'t have an autocomplete attribute',
38+
/**
39+
* @description Tooltip text shown in the Elements panel when an element has an error.
40+
*/
41+
formLabelHasNeitherForNorNestedInput: 'No label associated with a form field',
42+
/**
43+
* @description Tooltip text shown in the Elements panel when an element has an error.
44+
*/
45+
formLabelForMatchesNonExistingIdError: 'Incorrect use of <label for=FORM_ELEMENT>',
46+
/**
47+
* @description Tooltip text shown in the Elements panel when an element has an error.
48+
*/
49+
formInputHasWrongButWellIntendedAutocompleteValueError: 'Non-standard autocomplete attribute value',
50+
/**
51+
* @description Tooltip text shown in the Elements panel when an element has an error.
52+
*/
53+
disallowedSelectChild: 'Invalid element or text node within <select>',
54+
/**
55+
* @description Tooltip text shown in the Elements panel when an element has an error.
56+
*/
57+
disallowedOptGroupChild: 'Invalid element or text node within <optgroup>',
58+
/**
59+
* @description Tooltip text shown in the Elements panel when an element has an error.
60+
*/
61+
nonPhrasingContentOptionChild: 'Non-phrasing content used within an <option> element',
62+
/**
63+
* @description Tooltip text shown in the Elements panel when an element has an error.
64+
*/
65+
interactiveContentOptionChild: 'Interactive element inside of an <option> element',
66+
/**
67+
* @description Tooltip text shown in the Elements panel when an element has an error.
68+
*/
69+
interactiveContentLegendChild: 'Interactive element inside of a <legend> element',
70+
} as const;
71+
72+
const str_ = i18n.i18n.registerUIStrings('panels/elements/ElementIssueUtils.ts', UIStrings);
73+
const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
74+
75+
export interface ElementIssueDetails {
76+
tooltip: string;
77+
nodeId?: Protocol.DOM.BackendNodeId;
78+
attribute?: string;
79+
}
80+
81+
export function getElementIssueDetails(issue: IssuesManager.Issue.Issue): ElementIssueDetails|undefined {
82+
if (issue instanceof IssuesManager.GenericIssue.GenericIssue) {
83+
const issueDetails = issue.details();
84+
return {
85+
tooltip: getTooltipFromGenericIssue(issueDetails.errorType),
86+
nodeId: issueDetails.violatingNodeId,
87+
attribute: issueDetails.violatingNodeAttribute,
88+
};
89+
}
90+
if (issue instanceof IssuesManager.SelectElementAccessibilityIssue.SelectElementAccessibilityIssue) {
91+
const issueDetails = issue.details();
92+
return {
93+
tooltip: getTooltipFromSelectElementAccessibilityIssue(issueDetails.selectElementAccessibilityIssueReason),
94+
nodeId: issueDetails.nodeId,
95+
};
96+
}
97+
98+
return undefined;
99+
}
100+
101+
function getTooltipFromGenericIssue(errorType: Protocol.Audits.GenericIssueErrorType): string {
102+
switch (errorType) {
103+
case Protocol.Audits.GenericIssueErrorType.FormLabelForNameError:
104+
return i18nString(UIStrings.formLabelForNameError);
105+
case Protocol.Audits.GenericIssueErrorType.FormDuplicateIdForInputError:
106+
return i18nString(UIStrings.formDuplicateIdForInputError);
107+
case Protocol.Audits.GenericIssueErrorType.FormInputWithNoLabelError:
108+
return i18nString(UIStrings.formInputWithNoLabelError);
109+
case Protocol.Audits.GenericIssueErrorType.FormAutocompleteAttributeEmptyError:
110+
return i18nString(UIStrings.formAutocompleteAttributeEmptyError);
111+
case Protocol.Audits.GenericIssueErrorType.FormEmptyIdAndNameAttributesForInputError:
112+
return i18nString(UIStrings.formEmptyIdAndNameAttributesForInputError);
113+
case Protocol.Audits.GenericIssueErrorType.FormAriaLabelledByToNonExistingId:
114+
return i18nString(UIStrings.formAriaLabelledByToNonExistingId);
115+
case Protocol.Audits.GenericIssueErrorType.FormInputAssignedAutocompleteValueToIdOrNameAttributeError:
116+
return i18nString(UIStrings.formInputAssignedAutocompleteValueToIdOrNameAttributeError);
117+
case Protocol.Audits.GenericIssueErrorType.FormLabelHasNeitherForNorNestedInput:
118+
return i18nString(UIStrings.formLabelHasNeitherForNorNestedInput);
119+
case Protocol.Audits.GenericIssueErrorType.FormLabelForMatchesNonExistingIdError:
120+
return i18nString(UIStrings.formLabelForMatchesNonExistingIdError);
121+
case Protocol.Audits.GenericIssueErrorType.FormInputHasWrongButWellIntendedAutocompleteValueError:
122+
return i18nString(UIStrings.formInputHasWrongButWellIntendedAutocompleteValueError);
123+
default:
124+
return '';
125+
}
126+
}
127+
128+
function getTooltipFromSelectElementAccessibilityIssue(reason: Protocol.Audits.SelectElementAccessibilityIssueReason):
129+
string {
130+
switch (reason) {
131+
case Protocol.Audits.SelectElementAccessibilityIssueReason.DisallowedSelectChild:
132+
return i18nString(UIStrings.disallowedSelectChild);
133+
case Protocol.Audits.SelectElementAccessibilityIssueReason.DisallowedOptGroupChild:
134+
return i18nString(UIStrings.disallowedOptGroupChild);
135+
case Protocol.Audits.SelectElementAccessibilityIssueReason.NonPhrasingContentOptionChild:
136+
return i18nString(UIStrings.nonPhrasingContentOptionChild);
137+
case Protocol.Audits.SelectElementAccessibilityIssueReason.InteractiveContentOptionChild:
138+
return i18nString(UIStrings.interactiveContentOptionChild);
139+
case Protocol.Audits.SelectElementAccessibilityIssueReason.InteractiveContentLegendChild:
140+
return i18nString(UIStrings.interactiveContentLegendChild);
141+
default:
142+
return '';
143+
}
144+
}

‎front_end/panels/elements/ElementsTreeElement.ts‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import * as Emulation from '../emulation/emulation.js';
5454

5555
import * as ElementsComponents from './components/components.js';
5656
import {canGetJSPath, cssPath, jsPath, xPath} from './DOMPath.js';
57+
import {getElementIssueDetails} from './ElementIssueUtils.js';
5758
import {ElementsPanel} from './ElementsPanel.js';
5859
import {type ElementsTreeOutline, MappedCharToEntity, type UpdateRecord} from './ElementsTreeOutline.js';
5960
import {ImagePreviewPopover} from './ImagePreviewPopover.js';
@@ -262,8 +263,8 @@ export class ElementsTreeElement extends UI.TreeOutline.TreeElement {
262263
private hintElement?: HTMLElement;
263264
private aiButtonContainer?: HTMLElement;
264265
private contentElement: HTMLElement;
265-
#elementIssues = new Map<string, IssuesManager.GenericIssue.GenericIssue>();
266-
#nodeElementToIssue = new Map<Element, IssuesManager.GenericIssue.GenericIssue>();
266+
#elementIssues = new Map<string, IssuesManager.Issue.Issue>();
267+
#nodeElementToIssue = new Map<Element, IssuesManager.Issue.Issue>();
267268
#highlights: Range[] = [];
268269

269270
readonly tagTypeContext: TagTypeContext;
@@ -434,7 +435,7 @@ export class ElementsTreeElement extends UI.TreeOutline.TreeElement {
434435
}
435436
}
436437

437-
addIssue(newIssue: IssuesManager.GenericIssue.GenericIssue): void {
438+
addIssue(newIssue: IssuesManager.Issue.Issue): void {
438439
if (this.#elementIssues.has(newIssue.primaryKey())) {
439440
return;
440441
}
@@ -443,21 +444,24 @@ export class ElementsTreeElement extends UI.TreeOutline.TreeElement {
443444
this.#applyIssueStyleAndTooltip(newIssue);
444445
}
445446

446-
#applyIssueStyleAndTooltip(issue: IssuesManager.GenericIssue.GenericIssue): void {
447-
const issueDetails = issue.details();
447+
#applyIssueStyleAndTooltip(issue: IssuesManager.Issue.Issue): void {
448+
const elementIssueDetails = getElementIssueDetails(issue);
449+
if (!elementIssueDetails) {
450+
return;
451+
}
448452

449-
if (issueDetails.violatingNodeAttribute) {
450-
this.#highlightViolatingAttr(issueDetails.violatingNodeAttribute, issue);
453+
if (elementIssueDetails.attribute) {
454+
this.#highlightViolatingAttr(elementIssueDetails.attribute, issue);
451455
} else {
452456
this.#highlightTagAsViolating(issue);
453457
}
454458
}
455459

456-
get issuesByNodeElement(): Map<Element, IssuesManager.GenericIssue.GenericIssue> {
460+
get issuesByNodeElement(): Map<Element, IssuesManager.Issue.Issue> {
457461
return this.#nodeElementToIssue;
458462
}
459463

460-
#highlightViolatingAttr(name: string, issue: IssuesManager.GenericIssue.GenericIssue): void {
464+
#highlightViolatingAttr(name: string, issue: IssuesManager.Issue.Issue): void {
461465
const tag = this.listItemElement.getElementsByClassName('webkit-html-tag')[0];
462466
const attributes = tag.getElementsByClassName('webkit-html-attribute');
463467
for (const attribute of attributes) {
@@ -469,7 +473,7 @@ export class ElementsTreeElement extends UI.TreeOutline.TreeElement {
469473
}
470474
}
471475

472-
#highlightTagAsViolating(issue: IssuesManager.GenericIssue.GenericIssue): void {
476+
#highlightTagAsViolating(issue: IssuesManager.Issue.Issue): void {
473477
const tagElement = this.listItemElement.getElementsByClassName('webkit-html-tag-name')[0];
474478
tagElement.classList.add('violating-element');
475479
this.#nodeElementToIssue.set(tagElement, issue);

‎front_end/panels/elements/ElementsTreeOutline.test.ts‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,15 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
import * as Root from '../../core/root/root.js';
56
import * as SDK from '../../core/sdk/sdk.js';
67
import * as Protocol from '../../generated/protocol.js';
8+
import * as IssuesManager from '../../models/issues_manager/issues_manager.js';
79
import {createTarget} from '../../testing/EnvironmentHelpers.js';
810
import {
911
describeWithMockConnection,
1012
} from '../../testing/MockConnection.js';
13+
import {MockIssuesModel} from '../../testing/MockIssuesModel.js';
1114

1215
import * as Elements from './elements.js';
1316

@@ -19,6 +22,7 @@ describeWithMockConnection('ElementsTreeOutline', () => {
1922
beforeEach(() => {
2023
target = createTarget();
2124

25+
Root.Runtime.experiments.enableForTest(Root.Runtime.ExperimentName.HIGHLIGHT_ERRORS_ELEMENTS_PANEL);
2226
treeOutline = new Elements.ElementsTreeOutline.ElementsTreeOutline(/* omitRootDOMNode */ true);
2327
treeOutline.wireToDOMModel(target.model(SDK.DOMModel.DOMModel) as SDK.DOMModel.DOMModel);
2428

@@ -90,4 +94,98 @@ describeWithMockConnection('ElementsTreeOutline', () => {
9094
treeOutline.rootDOMNode = selectNode;
9195
assert.isNotNull(treeOutline.findTreeElement(pickerIconNode!));
9296
});
97+
98+
it('should add an element-related issue to the relevant tree element', async () => {
99+
const divNodePayload = {
100+
nodeId: 2 as Protocol.DOM.NodeId,
101+
parentId: 1 as Protocol.DOM.NodeId,
102+
backendNodeId: 2 as Protocol.DOM.BackendNodeId,
103+
nodeType: Node.ELEMENT_NODE,
104+
nodeName: 'DIV',
105+
childNodeCount: 0,
106+
localName: 'div',
107+
nodeValue: 'A div',
108+
};
109+
const rootNode = SDK.DOMModel.DOMNode.create(model, null, false, {
110+
nodeId: 1 as Protocol.DOM.NodeId,
111+
backendNodeId: 1 as Protocol.DOM.BackendNodeId,
112+
nodeType: Node.ELEMENT_NODE,
113+
nodeName: 'BODY',
114+
localName: 'body',
115+
nodeValue: 'Body',
116+
childNodeCount: 1,
117+
children: [divNodePayload],
118+
});
119+
assert.isNotNull(rootNode);
120+
treeOutline.rootDOMNode = rootNode;
121+
const divNode = rootNode.children()![0];
122+
assert.isNotNull(divNode);
123+
const treeElement = treeOutline.findTreeElement(divNode);
124+
assert.isNotNull(treeElement);
125+
const deferredDOMNodeStub = sinon.stub(SDK.DOMModel.DeferredDOMNode.prototype, 'resolvePromise').resolves(divNode);
126+
127+
const issuesManager = IssuesManager.IssuesManager.IssuesManager.instance();
128+
const mockModel = new MockIssuesModel([]) as unknown as SDK.IssuesModel.IssuesModel;
129+
130+
// Test that generic issue can be added to the tree element.
131+
{
132+
const inspectorIssue = {
133+
code: Protocol.Audits.InspectorIssueCode.GenericIssue,
134+
details: {
135+
genericIssueDetails: {
136+
errorType: Protocol.Audits.GenericIssueErrorType.FormLabelForNameError,
137+
frameId: 'main' as Protocol.Page.FrameId,
138+
violatingNodeId: 2 as Protocol.DOM.BackendNodeId,
139+
},
140+
},
141+
};
142+
const issue = IssuesManager.GenericIssue.GenericIssue.fromInspectorIssue(mockModel, inspectorIssue)[0];
143+
issuesManager.dispatchEventToListeners(
144+
IssuesManager.IssuesManager.Events.ISSUE_ADDED, {issuesModel: mockModel, issue});
145+
await deferredDOMNodeStub();
146+
const tagElement = treeElement.listItemElement.getElementsByClassName('webkit-html-tag-name')[0];
147+
assert.isTrue(tagElement.classList.contains('violating-element'));
148+
// Reset tag to prepare for subsequent tests.
149+
tagElement.classList.remove('violating-element');
150+
}
151+
152+
// Test that <select> issue can be added to the tree element.
153+
{
154+
const inspectorIssue = {
155+
code: Protocol.Audits.InspectorIssueCode.SelectElementAccessibilityIssue,
156+
details: {
157+
selectElementAccessibilityIssueDetails: {
158+
nodeId: 2 as Protocol.DOM.BackendNodeId,
159+
selectElementAccessibilityIssueReason:
160+
Protocol.Audits.SelectElementAccessibilityIssueReason.DisallowedSelectChild,
161+
hasDisallowedAttributes: false,
162+
},
163+
},
164+
};
165+
const issue = IssuesManager.SelectElementAccessibilityIssue.SelectElementAccessibilityIssue.fromInspectorIssue(
166+
mockModel, inspectorIssue)[0];
167+
issuesManager.dispatchEventToListeners(
168+
IssuesManager.IssuesManager.Events.ISSUE_ADDED, {issuesModel: mockModel, issue});
169+
await deferredDOMNodeStub();
170+
const tagElement = treeElement.listItemElement.getElementsByClassName('webkit-html-tag-name')[0];
171+
assert.isTrue(tagElement.classList.contains('violating-element'));
172+
// Reset tag to prepare for subsequent tests.
173+
tagElement.classList.remove('violating-element');
174+
}
175+
176+
// Test that non-supported issue won't be added to the tree element.
177+
{
178+
const inspectorIssue = {
179+
code: Protocol.Audits.InspectorIssueCode.ContentSecurityPolicyIssue,
180+
details: {},
181+
};
182+
const issue = IssuesManager.ContentSecurityPolicyIssue.ContentSecurityPolicyIssue.fromInspectorIssue(
183+
mockModel, inspectorIssue)[0];
184+
issuesManager.dispatchEventToListeners(
185+
IssuesManager.IssuesManager.Events.ISSUE_ADDED, {issuesModel: mockModel, issue});
186+
await deferredDOMNodeStub();
187+
const tagElement = treeElement.listItemElement.getElementsByClassName('webkit-html-tag-name')[0];
188+
assert.isFalse(tagElement.classList.contains('violating-element'));
189+
}
190+
});
93191
});

0 commit comments

Comments
 (0)