diff --git a/front_end/issues/IssueAggregator.js b/front_end/issues/IssueAggregator.js index 78e1c9c6ac..c5cebc67fb 100644 --- a/front_end/issues/IssueAggregator.js +++ b/front_end/issues/IssueAggregator.js @@ -27,8 +27,8 @@ export class AggregatedIssue extends SDK.Issue.Issue { this._mixedContents = new Map(); /** @type {!Map} */ this._heavyAdIssueDetails = new Map(); - /** @type {!Set} */ - this._cspViolations = new Set(); + /** @type {!Set} */ + this._cspIssues = new Set(); /** @type {!Map} */ this._blockedByResponseDetails = new Map(); this._aggregatedIssuesCount = 0; @@ -82,11 +82,10 @@ export class AggregatedIssue extends SDK.Issue.Issue { } /** - * @override - * @returns {!Set} + * @returns {!Iterable} */ - cspViolations() { - return this._cspViolations; + cspIssues() { + return this._cspIssues; } /** @@ -164,13 +163,13 @@ export class AggregatedIssue extends SDK.Issue.Issue { const key = JSON.stringify(heavyAds); this._heavyAdIssueDetails.set(key, heavyAds); } - for (const cspViolation of issue.cspViolations()) { - this._cspViolations.add(cspViolation); - } for (const details of issue.blockedByResponseDetails()) { const key = JSON.stringify(details, ['parentFrame', 'blockedFrame', 'requestId', 'frameId', 'reason', 'request']); this._blockedByResponseDetails.set(key, details); } + if (issue instanceof SDK.ContentSecurityPolicyIssue.ContentSecurityPolicyIssue) { + this._cspIssues.add(issue); + } } } diff --git a/front_end/issues/IssuesPane.js b/front_end/issues/IssuesPane.js index d46f97af69..c6b0405314 100644 --- a/front_end/issues/IssuesPane.js +++ b/front_end/issues/IssuesPane.js @@ -410,28 +410,33 @@ class AffectedDirectivesView extends AffectedResourcesView { /** * @param {!Element} element * @param {number | undefined} nodeId + * @param {?SDK.SDKModel.Target} target */ - _appendBlockedElement(element, nodeId) { + _appendBlockedElement(element, nodeId, target) { const violatingNode = document.createElement('td'); violatingNode.classList.add('affected-resource-csp-info-node'); if (nodeId) { const violatingNodeId = nodeId; const icon = UI.Icon.Icon.create('largeicon-node-search', 'icon'); + icon.classList.add('element-reveal-icon'); - const target = /** @type {!SDK.SDKModel.Target} */ (SDK.SDKModel.TargetManager.instance().mainTarget()); icon.onclick = () => { - const deferredDOMNode = new SDK.DOMModel.DeferredDOMNode(target, violatingNodeId); - Common.Revealer.reveal(deferredDOMNode); + if (target) { + const deferredDOMNode = new SDK.DOMModel.DeferredDOMNode(target, violatingNodeId); + Common.Revealer.reveal(deferredDOMNode); + } }; UI.Tooltip.Tooltip.install(icon, ls`Click to reveal the violating DOM node in the Elements panel`); violatingNode.appendChild(icon); violatingNode.onmouseenter = () => { - const deferredDOMNode = new SDK.DOMModel.DeferredDOMNode(target, violatingNodeId); - if (deferredDOMNode) { - deferredDOMNode.highlight(); + if (target) { + const deferredDOMNode = new SDK.DOMModel.DeferredDOMNode(target, violatingNodeId); + if (deferredDOMNode) { + deferredDOMNode.highlight(); + } } }; violatingNode.onmouseleave = () => SDK.OverlayModel.OverlayModel.hideDOMNodeHighlight(); @@ -457,9 +462,9 @@ class AffectedDirectivesView extends AffectedResourcesView { } /** - * @param {!Set} cspViolations + * @param {!Iterable} cspIssues */ - _appendAffectedContentSecurityPolicyDetails(cspViolations) { + _appendAffectedContentSecurityPolicyDetails(cspIssues) { const header = document.createElement('tr'); if (this._issue.code() === SDK.ContentSecurityPolicyIssue.inlineViolationCode) { this._appendDirectiveColumnTitle(header); @@ -481,34 +486,35 @@ class AffectedDirectivesView extends AffectedResourcesView { } this._affectedResources.appendChild(header); let count = 0; - for (const cspViolation of cspViolations) { + for (const cspIssue of cspIssues) { count++; - this._appendAffectedContentSecurityPolicyDetail(cspViolation); + this._appendAffectedContentSecurityPolicyDetail(cspIssue); } this.updateAffectedResourceCount(count); } /** - * @param {!Protocol.Audits.ContentSecurityPolicyIssueDetails} cspViolation + * @param {!SDK.ContentSecurityPolicyIssue.ContentSecurityPolicyIssue} cspIssue */ - _appendAffectedContentSecurityPolicyDetail(cspViolation) { + _appendAffectedContentSecurityPolicyDetail(cspIssue) { const element = document.createElement('tr'); element.classList.add('affected-resource-directive'); + const cspIssueDetails = cspIssue.details(); if (this._issue.code() === SDK.ContentSecurityPolicyIssue.inlineViolationCode) { - this._appendViolatedDirective(element, cspViolation.violatedDirective); - this._appendBlockedElement(element, cspViolation.violatingNodeId); - this._appendSourceLocation(element, cspViolation.sourceCodeLocation); + this._appendViolatedDirective(element, cspIssueDetails.violatedDirective); + this._appendBlockedElement(element, cspIssueDetails.violatingNodeId, cspIssue.model().getTargetIfNotDisposed()); + this._appendSourceLocation(element, cspIssueDetails.sourceCodeLocation); this._appendBlockedStatus(element); } else if (this._issue.code() === SDK.ContentSecurityPolicyIssue.urlViolationCode) { - const url = cspViolation.blockedURL ? cspViolation.blockedURL : ''; + const url = cspIssueDetails.blockedURL ? cspIssueDetails.blockedURL : ''; this._appendBlockedURL(element, url); this._appendBlockedStatus(element); - this._appendViolatedDirective(element, cspViolation.violatedDirective); - this._appendSourceLocation(element, cspViolation.sourceCodeLocation); + this._appendViolatedDirective(element, cspIssueDetails.violatedDirective); + this._appendSourceLocation(element, cspIssueDetails.sourceCodeLocation); } else if (this._issue.code() === SDK.ContentSecurityPolicyIssue.evalViolationCode) { - this._appendSourceLocation(element, cspViolation.sourceCodeLocation); - this._appendViolatedDirective(element, cspViolation.violatedDirective); + this._appendSourceLocation(element, cspIssueDetails.sourceCodeLocation); + this._appendViolatedDirective(element, cspIssueDetails.violatedDirective); this._appendBlockedStatus(element); } else { return; @@ -522,7 +528,7 @@ class AffectedDirectivesView extends AffectedResourcesView { */ update() { this.clear(); - this._appendAffectedContentSecurityPolicyDetails(this._issue.cspViolations()); + this._appendAffectedContentSecurityPolicyDetails(this._issue.cspIssues()); } } diff --git a/front_end/sdk/ContentSecurityPolicyIssue.js b/front_end/sdk/ContentSecurityPolicyIssue.js index a3ff92343b..509016d679 100644 --- a/front_end/sdk/ContentSecurityPolicyIssue.js +++ b/front_end/sdk/ContentSecurityPolicyIssue.js @@ -3,19 +3,22 @@ // found in the LICENSE file. import {ls} from '../platform/platform.js'; - import {Issue, IssueCategory, IssueDescription, IssueKind} from './Issue.js'; // eslint-disable-line no-unused-vars +import {IssuesModel} from './IssuesModel.js'; // eslint-disable-line no-unused-vars + export class ContentSecurityPolicyIssue extends Issue { /** * @param {!Protocol.Audits.ContentSecurityPolicyIssueDetails} issueDetails + * @param {!IssuesModel} issuesModel */ - constructor(issueDetails) { + constructor(issueDetails, issuesModel) { const issue_code = [ Protocol.Audits.InspectorIssueCode.ContentSecurityPolicyIssue, issueDetails.contentSecurityPolicyViolationType ].join('::'); super(issue_code); this._issueDetails = issueDetails; + this._issuesModel = issuesModel; } /** @@ -50,11 +53,17 @@ export class ContentSecurityPolicyIssue extends Issue { } /** - * @override - * @returns {!Iterable} + * @returns {!IssuesModel} */ - cspViolations() { - return [this._issueDetails]; + model() { + return this._issuesModel; + } + + /** + * @returns {!Protocol.Audits.ContentSecurityPolicyIssueDetails} + */ + details() { + return this._issueDetails; } } diff --git a/front_end/sdk/Issue.js b/front_end/sdk/Issue.js index 608dc18f89..db4034611f 100644 --- a/front_end/sdk/Issue.js +++ b/front_end/sdk/Issue.js @@ -126,13 +126,6 @@ export class Issue extends Common.ObjectWrapper.ObjectWrapper { return []; } - /** - * @returns {!Iterable} - */ - cspViolations() { - return []; - } - /** * @return {!Iterable} */ diff --git a/front_end/sdk/IssuesModel.js b/front_end/sdk/IssuesModel.js index 2f820efb26..57e3eac21a 100644 --- a/front_end/sdk/IssuesModel.js +++ b/front_end/sdk/IssuesModel.js @@ -28,6 +28,7 @@ export class IssuesModel extends SDKModel { /** @type {*} */ this._auditsAgent = null; this.ensureEnabled(); + this._disposed = false; } /** @@ -81,6 +82,24 @@ export class IssuesModel extends SDKModel { console.warn(`No handler registered for issue code ${inspectorIssue.code}`); return []; } + + /** + * @override + */ + dispose() { + super.dispose(); + this._disposed = true; + } + + /** + * @returns {?Target} + */ + getTargetIfNotDisposed() { + if (!this._disposed) { + return this.target(); + } + return null; + } } /** @@ -124,7 +143,7 @@ function createIssuesForContentSecurityPolicyIssue(issuesModel, inspectorDetails console.warn('Content security policy issue without details received.'); return []; } - return [new ContentSecurityPolicyIssue(cspDetails)]; + return [new ContentSecurityPolicyIssue(cspDetails, issuesModel)]; } diff --git a/test/e2e/elements/BUILD.gn b/test/e2e/elements/BUILD.gn index fa77ff21bb..8107fed651 100644 --- a/test/e2e/elements/BUILD.gn +++ b/test/e2e/elements/BUILD.gn @@ -12,6 +12,7 @@ node_ts_library("elements") { "element-breadcrumbs_test.ts", "layout-pane_test.ts", "pseudo-states_test.ts", + "reveal-correct-node_test.ts", "selection-after-delete_test.ts", "shadowroot-styles_test.ts", "sidebar-event-listeners-remove_test.ts", diff --git a/test/e2e/elements/reveal-correct-node_test.ts b/test/e2e/elements/reveal-correct-node_test.ts new file mode 100644 index 0000000000..af6bde1a81 --- /dev/null +++ b/test/e2e/elements/reveal-correct-node_test.ts @@ -0,0 +1,22 @@ +// Copyright 2020 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import {goToResource} from '../../shared/helper.js'; +import {describe, it} from '../../shared/mocha-extensions.js'; +import {waitForSelectedTreeElementSelectorWhichIncludesText} from '../helpers/elements-helpers.js'; +import {expandIssue, navigateToIssuesTab, revealNodeInElementsPanel} from '../helpers/issues-helpers.js'; + +// TODO: Add a second node reveal test, where am issue is produced by an OOPIF + +describe('The Issues tab', async () => { + it('should reveal an element in the Elements panelwhen the node icon is clicked', async () => { + await goToResource('elements/element-reveal-inline-issue.html'); + + await navigateToIssuesTab(); + await expandIssue(); + await revealNodeInElementsPanel(); + + await waitForSelectedTreeElementSelectorWhichIncludesText('alert("This should be blocked by CSP");'); + }); +}); diff --git a/test/e2e/helpers/BUILD.gn b/test/e2e/helpers/BUILD.gn index 5ba85bdbb5..e4740bb4f0 100644 --- a/test/e2e/helpers/BUILD.gn +++ b/test/e2e/helpers/BUILD.gn @@ -16,6 +16,7 @@ node_ts_library("helpers") { "elements-helpers.ts", "emulation-helpers.ts", "event-listeners-helpers.ts", + "issues-helpers.ts", "layers-helpers.ts", "lighthouse-helpers.ts", "media-helpers.ts", diff --git a/test/e2e/helpers/elements-helpers.ts b/test/e2e/helpers/elements-helpers.ts index e431281adb..6c006195b8 100644 --- a/test/e2e/helpers/elements-helpers.ts +++ b/test/e2e/helpers/elements-helpers.ts @@ -110,6 +110,14 @@ export const waitForSelectedTreeElementSelectorWithTextcontent = async (expected }); }; +export const waitForSelectedTreeElementSelectorWhichIncludesText = async (expectedTextContent: string) => { + await waitForFunction(async () => { + const selectedNode = await waitFor(SELECTED_TREE_ELEMENT_SELECTOR); + const selectedTextContent = await selectedNode.evaluate(node => node.textContent); + return selectedTextContent && selectedTextContent.includes(expectedTextContent); + }); +}; + export const waitForChildrenOfSelectedElementNode = async () => { await waitFor(`${SELECTED_TREE_ELEMENT_SELECTOR} + ol > li`); }; diff --git a/test/e2e/helpers/issues-helpers.ts b/test/e2e/helpers/issues-helpers.ts new file mode 100644 index 0000000000..7311119472 --- /dev/null +++ b/test/e2e/helpers/issues-helpers.ts @@ -0,0 +1,26 @@ +// Copyright 2020 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import {click, waitFor} from '../../shared/helper.js'; +import {openPanelViaMoreTools} from './settings-helpers'; + +export const ISSUE = '.issue'; +export const AFFECTED_ELEMENT_ICON = '.affected-resource-csp-info-node'; +export const ELEMENT_REVEAL_ICON = '.element-reveal-icon'; +export const ELEMENTS_PANEL_SELECTOR = '.panel[aria-label="elements"]'; + +export async function navigateToIssuesTab() { + await openPanelViaMoreTools('Issues'); +} + +export async function expandIssue() { + await waitFor(ISSUE); + await click(ISSUE); + await waitFor('.message'); +} + +export async function revealNodeInElementsPanel() { + const revealIcon = await waitFor(ELEMENT_REVEAL_ICON); + await revealIcon.click(); +} diff --git a/test/e2e/resources/elements/BUILD.gn b/test/e2e/resources/elements/BUILD.gn index 83c098d13c..32c836dc78 100644 --- a/test/e2e/resources/elements/BUILD.gn +++ b/test/e2e/resources/elements/BUILD.gn @@ -10,6 +10,7 @@ copy_to_gen("elements") { "css-grid.html", "css-variables.html", "element-breadcrumbs.html", + "element-reveal-inline-issue.html", "focus.html", "hover.html", "multiple-constructed-stylesheets.html", diff --git a/test/e2e/resources/elements/element-reveal-inline-issue.html b/test/e2e/resources/elements/element-reveal-inline-issue.html new file mode 100644 index 0000000000..0a6fe1af8c --- /dev/null +++ b/test/e2e/resources/elements/element-reveal-inline-issue.html @@ -0,0 +1,15 @@ + + + + + +

Webpage with an oopif causing an issue of inline violation type

+ +
+ +
+ + + \ No newline at end of file