diff --git a/src/DetailsView/components/iframe-skipped-warning.tsx b/src/DetailsView/components/iframe-skipped-warning.tsx new file mode 100644 index 00000000000..42f99b87ab5 --- /dev/null +++ b/src/DetailsView/components/iframe-skipped-warning.tsx @@ -0,0 +1,18 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +import { NewTabLink } from 'common/components/new-tab-link'; +import { NamedFC } from 'common/react/named-fc'; +import * as React from 'react'; + +export const IframeSkippedWarningContainerAutomationId = 'iframe-skipped-warning-container'; + +export const IframeSkippedWarning = NamedFC('IframeSkippedWarning', () => ( +
+ There are iframes in the target page that are non-responsive. These frames have been skipped + and are not included in results. If you get this warning unexpectedly in a page where you + expect the iframes to be responsive, please + + let us know. + +
+)); diff --git a/src/DetailsView/components/warning-configuration.tsx b/src/DetailsView/components/warning-configuration.tsx index 511ebcdbbdb..86d0541a81e 100644 --- a/src/DetailsView/components/warning-configuration.tsx +++ b/src/DetailsView/components/warning-configuration.tsx @@ -2,6 +2,7 @@ // Licensed under the MIT License. import { ReactFCWithDisplayName } from 'common/react/named-fc'; import { ScanIncompleteWarningId } from 'common/types/store-data/scan-incomplete-warnings'; +import { IframeSkippedWarning } from 'DetailsView/components/iframe-skipped-warning'; import { AssessmentIframeWarning, AssessmentIframeWarningDeps, @@ -24,8 +25,10 @@ export type WarningConfiguration = { export const assessmentWarningConfiguration: WarningConfiguration = { 'missing-required-cross-origin-permissions': AssessmentIframeWarning, + 'frame-skipped': IframeSkippedWarning, }; export const fastpassWarningConfiguration: WarningConfiguration = { 'missing-required-cross-origin-permissions': FastPassIframeWarning, + 'frame-skipped': IframeSkippedWarning, }; diff --git a/src/common/types/store-data/scan-incomplete-warnings.ts b/src/common/types/store-data/scan-incomplete-warnings.ts index d70e20db9cc..93905f55812 100644 --- a/src/common/types/store-data/scan-incomplete-warnings.ts +++ b/src/common/types/store-data/scan-incomplete-warnings.ts @@ -2,4 +2,4 @@ // Licensed under the MIT License. // This is a string-enum, but we only have one type right now -export type ScanIncompleteWarningId = 'missing-required-cross-origin-permissions'; +export type ScanIncompleteWarningId = 'missing-required-cross-origin-permissions' | 'frame-skipped'; diff --git a/src/injected/analyzers/base-analyzer.ts b/src/injected/analyzers/base-analyzer.ts index 86e618aa2dc..2c821b495a7 100644 --- a/src/injected/analyzers/base-analyzer.ts +++ b/src/injected/analyzers/base-analyzer.ts @@ -49,7 +49,7 @@ export class BaseAnalyzer implements Analyzer { scanResult: originalAxeResult, testType: config.testType, scanIncompleteWarnings: - this.scanIncompleteWarningDetector.detectScanIncompleteWarnings(), + this.scanIncompleteWarningDetector.detectScanIncompleteWarnings(originalAxeResult), }; return { diff --git a/src/injected/analyzers/notification-text-creator.ts b/src/injected/analyzers/notification-text-creator.ts index a5bdeff9b1b..2b565c5a07c 100644 --- a/src/injected/analyzers/notification-text-creator.ts +++ b/src/injected/analyzers/notification-text-creator.ts @@ -16,7 +16,7 @@ export class NotificationTextCreator { }; public needsReviewText: TextGenerator = (results: UnifiedResult[]) => { - const warnings = this.scanIncompleteWarningDetector.detectScanIncompleteWarnings(); + const warnings = this.scanIncompleteWarningDetector.detectScanIncompleteWarnings(null); if (isEmpty(results) && isEmpty(warnings)) { return 'Congratulations!\n\nNeeds review found no instances to review on this page.'; } diff --git a/src/injected/analyzers/unified-result-sender.ts b/src/injected/analyzers/unified-result-sender.ts index 8953db079c9..6627d380674 100644 --- a/src/injected/analyzers/unified-result-sender.ts +++ b/src/injected/analyzers/unified-result-sender.ts @@ -64,7 +64,7 @@ export class UnifiedResultSender { notificationMessage: TextGenerator, ): UnifiedScanCompletedPayload => { const scanIncompleteWarnings = - this.scanIncompleteWarningDetector.detectScanIncompleteWarnings(); + this.scanIncompleteWarningDetector.detectScanIncompleteWarnings(results); let telemetry: ScanIncompleteWarningsTelemetryData = null; diff --git a/src/injected/scan-incomplete-warning-detector.ts b/src/injected/scan-incomplete-warning-detector.ts index fdc4cbd2889..c4848efeb0b 100644 --- a/src/injected/scan-incomplete-warning-detector.ts +++ b/src/injected/scan-incomplete-warning-detector.ts @@ -3,6 +3,7 @@ import { BaseStore } from 'common/base-store'; import { PermissionsStateStoreData } from 'common/types/store-data/permissions-state-store-data'; import { ScanIncompleteWarningId } from 'common/types/store-data/scan-incomplete-warnings'; +import { ScanResults } from 'scanner/iruleresults'; import { IframeDetector } from './iframe-detector'; export class ScanIncompleteWarningDetector { @@ -11,14 +12,18 @@ export class ScanIncompleteWarningDetector { private permissionsStateStore: BaseStore>, ) {} - public detectScanIncompleteWarnings = () => { + public detectScanIncompleteWarnings = (results: ScanResults | null) => { const warnings: ScanIncompleteWarningId[] = []; + if ( this.iframeDetector.hasIframes() && !this.permissionsStateStore.getState().hasAllUrlAndFilePermissions ) { warnings.push('missing-required-cross-origin-permissions'); + } else if (results && results.framesSkipped) { + warnings.push('frame-skipped'); } + return warnings; }; } diff --git a/src/scanner/get-rule-inclusions.ts b/src/scanner/get-rule-inclusions.ts index 6a23654c8c5..c47be42fd46 100644 --- a/src/scanner/get-rule-inclusions.ts +++ b/src/scanner/get-rule-inclusions.ts @@ -4,7 +4,10 @@ import { IRuleConfiguration } from 'scanner/iruleresults'; import { DictionaryStringTo } from 'types/common-types'; export type RuleIncluded = + // Rules included in automated checks by default | { status: 'included'; reason?: string } + // Rules included when running any scan + | { status: 'included-always'; reason: string } | { status: 'excluded'; reason: string }; export const explicitRuleOverrides: DictionaryStringTo = { @@ -36,6 +39,10 @@ export const explicitRuleOverrides: DictionaryStringTo = { status: 'included', reason: 'best practice rule that was investigated with no known false positives, implemented as an automated check.', }, + 'frame-tested': { + status: 'included-always', + reason: 'Tests for unresponsive frames, enables iframe skipped warning', + }, }; // all the rules we enable in needs review diff --git a/src/scanner/iruleresults.d.ts b/src/scanner/iruleresults.d.ts index b746305eea1..85f7911b33e 100644 --- a/src/scanner/iruleresults.d.ts +++ b/src/scanner/iruleresults.d.ts @@ -91,6 +91,7 @@ export interface ScanResults { timestamp: string; targetPageUrl: string; targetPageTitle: string; + framesSkipped?: boolean; } export interface RuleDecorations { diff --git a/src/scanner/result-decorator.ts b/src/scanner/result-decorator.ts index 411e239a522..815e841c283 100644 --- a/src/scanner/result-decorator.ts +++ b/src/scanner/result-decorator.ts @@ -29,6 +29,7 @@ export class ResultDecorator { timestamp: results.timestamp, targetPageUrl: results.url, targetPageTitle: this.documentUtils.title(), + framesSkipped: results.incomplete.some(result => result.id === 'frame-tested'), }; return scanResults; diff --git a/src/scanner/scan-parameter-generator.ts b/src/scanner/scan-parameter-generator.ts index ecb905120e7..a7917649d35 100644 --- a/src/scanner/scan-parameter-generator.ts +++ b/src/scanner/scan-parameter-generator.ts @@ -21,10 +21,16 @@ export class ScanParameterGenerator { if (options == null || options.testsToRun == null) { result.runOnly!.values = Object.keys(this.rulesIncluded).filter( - id => this.rulesIncluded[id].status === 'included', + id => + this.rulesIncluded[id].status === 'included' || + this.rulesIncluded[id].status === 'included-always', ); } else { - result.runOnly!.values = options.testsToRun; + result.runOnly!.values = options.testsToRun.concat( + Object.keys(this.rulesIncluded).filter( + id => this.rulesIncluded[id].status === 'included-always', + ), + ); } return result; diff --git a/src/tests/unit/tests/DetailsView/components/__snapshots__/iframe-skipped-warning.test.tsx.snap b/src/tests/unit/tests/DetailsView/components/__snapshots__/iframe-skipped-warning.test.tsx.snap new file mode 100644 index 00000000000..0ede28134f6 --- /dev/null +++ b/src/tests/unit/tests/DetailsView/components/__snapshots__/iframe-skipped-warning.test.tsx.snap @@ -0,0 +1,14 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`IframeSkippedWarning render 1`] = ` +
+ There are iframes in the target page that are non-responsive. These frames have been skipped and are not included in results. If you get this warning unexpectedly in a page where you expect the iframes to be responsive, please + + let us know. + +
+`; diff --git a/src/tests/unit/tests/DetailsView/components/iframe-skipped-warning.test.tsx b/src/tests/unit/tests/DetailsView/components/iframe-skipped-warning.test.tsx new file mode 100644 index 00000000000..465e4978600 --- /dev/null +++ b/src/tests/unit/tests/DetailsView/components/iframe-skipped-warning.test.tsx @@ -0,0 +1,12 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +import { IframeSkippedWarning } from 'DetailsView/components/iframe-skipped-warning'; +import { shallow } from 'enzyme'; +import * as React from 'react'; + +describe('IframeSkippedWarning', () => { + test('render', () => { + const wrapper = shallow(); + expect(wrapper.getElement()).toMatchSnapshot(); + }); +}); diff --git a/src/tests/unit/tests/DetailsView/components/scan-incomplete-warning.test.tsx b/src/tests/unit/tests/DetailsView/components/scan-incomplete-warning.test.tsx index d85d80143ce..bfdbe2df5c4 100644 --- a/src/tests/unit/tests/DetailsView/components/scan-incomplete-warning.test.tsx +++ b/src/tests/unit/tests/DetailsView/components/scan-incomplete-warning.test.tsx @@ -26,6 +26,7 @@ describe('ScanIncompleteWarning', () => { NamedFC('test', _ => null); warningConfiguration = { 'missing-required-cross-origin-permissions': scanIncompleteWarningStub, + 'frame-skipped': scanIncompleteWarningStub, }; }); diff --git a/src/tests/unit/tests/injected/analyzers/base-analyzer.test.ts b/src/tests/unit/tests/injected/analyzers/base-analyzer.test.ts index 0a668da53c0..0f4622f7def 100644 --- a/src/tests/unit/tests/injected/analyzers/base-analyzer.test.ts +++ b/src/tests/unit/tests/injected/analyzers/base-analyzer.test.ts @@ -51,7 +51,7 @@ describe('BaseAnalyzer', () => { }; scanIncompleteWarningDetectorMock - .setup(idm => idm.detectScanIncompleteWarnings()) + .setup(idm => idm.detectScanIncompleteWarnings(undefined)) .returns(() => scanIncompleteWarnings); sendMessageMock diff --git a/src/tests/unit/tests/injected/analyzers/batched-rule-analyzer.test.ts b/src/tests/unit/tests/injected/analyzers/batched-rule-analyzer.test.ts index 7eff7a1a9b5..9653467e264 100644 --- a/src/tests/unit/tests/injected/analyzers/batched-rule-analyzer.test.ts +++ b/src/tests/unit/tests/injected/analyzers/batched-rule-analyzer.test.ts @@ -65,7 +65,7 @@ describe('BatchedRuleAnalyzer', () => { .returns(() => scopingState) .verifiable(); scanIncompleteWarningDetectorMock - .setup(idm => idm.detectScanIncompleteWarnings()) + .setup(idm => idm.detectScanIncompleteWarnings(It.isAny())) .returns(() => []); }); diff --git a/src/tests/unit/tests/injected/analyzers/notification-text-creator.test.ts b/src/tests/unit/tests/injected/analyzers/notification-text-creator.test.ts index 0f40c2c319f..2deaeb5d8a2 100644 --- a/src/tests/unit/tests/injected/analyzers/notification-text-creator.test.ts +++ b/src/tests/unit/tests/injected/analyzers/notification-text-creator.test.ts @@ -42,7 +42,7 @@ describe('NotificationTextCreator', () => { 'generates text for needs review with results: $unifiedResults and warnings: $testScanWarnings', ({ unifiedResults, testScanWarnings, expectedText }) => { scanIncompleteWarningDetectorMock - .setup(m => m.detectScanIncompleteWarnings()) + .setup(m => m.detectScanIncompleteWarnings(null)) .returns(() => testScanWarnings); expect(testSubject.needsReviewText(unifiedResults)).toEqual(expectedText); @@ -64,7 +64,7 @@ describe('NotificationTextCreator', () => { 'generates null for automated checks with $unifiedResults and $testScanWarnings', // automated checks uses notifications generated from visualizationMessages.Common.ScanCompleted ({ unifiedResults, testScanWarnings }) => { scanIncompleteWarningDetectorMock - .setup(m => m.detectScanIncompleteWarnings()) + .setup(m => m.detectScanIncompleteWarnings(null)) .returns(testScanWarnings); expect(testSubject.automatedChecksText(unifiedResults)).toEqual(null); diff --git a/src/tests/unit/tests/injected/analyzers/rule-analyzer.test.ts b/src/tests/unit/tests/injected/analyzers/rule-analyzer.test.ts index 1afef14f35e..c04ac759ab8 100644 --- a/src/tests/unit/tests/injected/analyzers/rule-analyzer.test.ts +++ b/src/tests/unit/tests/injected/analyzers/rule-analyzer.test.ts @@ -74,7 +74,7 @@ describe('RuleAnalyzer', () => { .returns(() => scopingState) .verifiable(); scanIncompleteWarningDetectorMock - .setup(idm => idm.detectScanIncompleteWarnings()) + .setup(idm => idm.detectScanIncompleteWarnings(It.isAny())) .returns(() => []); }); diff --git a/src/tests/unit/tests/injected/analyzers/tab-stops-analyzer.test.ts b/src/tests/unit/tests/injected/analyzers/tab-stops-analyzer.test.ts index 8cf94716ca0..db88a14a9a0 100644 --- a/src/tests/unit/tests/injected/analyzers/tab-stops-analyzer.test.ts +++ b/src/tests/unit/tests/injected/analyzers/tab-stops-analyzer.test.ts @@ -64,7 +64,7 @@ describe('TabStopsAnalyzer', () => { tabStopsRequirementResultProcessorMock = Mock.ofType(); scanIncompleteWarningDetectorMock - .setup(idm => idm.detectScanIncompleteWarnings()) + .setup(idm => idm.detectScanIncompleteWarnings(It.isAny())) .returns(() => []); testSubject = new TabStopsAnalyzer( diff --git a/src/tests/unit/tests/injected/analyzers/unified-result-sender.test.ts b/src/tests/unit/tests/injected/analyzers/unified-result-sender.test.ts index f1938e54c85..ebe2a48f491 100644 --- a/src/tests/unit/tests/injected/analyzers/unified-result-sender.test.ts +++ b/src/tests/unit/tests/injected/analyzers/unified-result-sender.test.ts @@ -111,7 +111,7 @@ describe('sendConvertedResults', () => { .setup(m => m(inputResults)) .returns(val => unifiedRules); scanIncompleteWarningDetectorMock - .setup(m => m.detectScanIncompleteWarnings()) + .setup(m => m.detectScanIncompleteWarnings(inputResults)) .returns(() => warnings); filterNeedsReviewResultsMock .setup(m => m(axeInputResults)) diff --git a/src/tests/unit/tests/injected/scan-incomplete-warning-detector.test.ts b/src/tests/unit/tests/injected/scan-incomplete-warning-detector.test.ts index 6d5be180071..ed41de9811e 100644 --- a/src/tests/unit/tests/injected/scan-incomplete-warning-detector.test.ts +++ b/src/tests/unit/tests/injected/scan-incomplete-warning-detector.test.ts @@ -4,6 +4,7 @@ import { BaseStore } from 'common/base-store'; import { PermissionsStateStoreData } from 'common/types/store-data/permissions-state-store-data'; import { IframeDetector } from 'injected/iframe-detector'; import { ScanIncompleteWarningDetector } from 'injected/scan-incomplete-warning-detector'; +import { ScanResults } from 'scanner/iruleresults'; import { IMock, Mock } from 'typemoq'; describe('ScanIncompleteWarningDetector', () => { @@ -35,7 +36,33 @@ describe('ScanIncompleteWarningDetector', () => { .setup(m => m.getState()) .returns(() => ({ hasAllUrlAndFilePermissions })); - expect(testSubject.detectScanIncompleteWarnings()).toStrictEqual(expectedResults); + expect(testSubject.detectScanIncompleteWarnings(null)).toStrictEqual(expectedResults); + }, + ); + + it('should not detect frames skipped warning when it detects permissions warning ', () => { + iframeDetectorMock.setup(m => m.hasIframes()).returns(() => true); + permissionsStateStoreMock + .setup(m => m.getState()) + .returns(() => ({ hasAllUrlAndFilePermissions: false })); + const results = { framesSkipped: true } as ScanResults; + expect(testSubject.detectScanIncompleteWarnings(results)).toStrictEqual([ + 'missing-required-cross-origin-permissions', + ]); + }); + + it('should detect no frames skipped if results are null', () => { + expect(testSubject.detectScanIncompleteWarnings(null)).toStrictEqual([]); + }); + + it.each([true, false])( + 'should detect frames skipped correctly if results frames skipped is %s', + (resultsFramesSkipped: boolean) => { + const results = { framesSkipped: resultsFramesSkipped } as ScanResults; + + expect(testSubject.detectScanIncompleteWarnings(results)).toStrictEqual( + resultsFramesSkipped ? ['frame-skipped'] : [], + ); }, ); }); diff --git a/src/tests/unit/tests/scanner/__snapshots__/get-rule-inclusions.test.ts.snap b/src/tests/unit/tests/scanner/__snapshots__/get-rule-inclusions.test.ts.snap index 9d34a756780..c33c7b80bcb 100644 --- a/src/tests/unit/tests/scanner/__snapshots__/get-rule-inclusions.test.ts.snap +++ b/src/tests/unit/tests/scanner/__snapshots__/get-rule-inclusions.test.ts.snap @@ -142,8 +142,8 @@ exports[`getRuleInclusions matches snapshotted list of production rules 1`] = ` "status": "included", }, "frame-tested": { - "reason": "rule is tagged best-practice", - "status": "excluded", + "reason": "Tests for unresponsive frames, enables iframe skipped warning", + "status": "included-always", }, "frame-title": { "status": "included", diff --git a/src/tests/unit/tests/scanner/result-decorator.test.ts b/src/tests/unit/tests/scanner/result-decorator.test.ts index c80966dad88..af9ced11a4d 100644 --- a/src/tests/unit/tests/scanner/result-decorator.test.ts +++ b/src/tests/unit/tests/scanner/result-decorator.test.ts @@ -88,6 +88,7 @@ describe('ResultDecorator', () => { timestamp: 100, targetPageTitle: 'test title', targetPageUrl: 'https://test_url', + framesSkipped: false, }; messageDecoratorMock @@ -168,6 +169,7 @@ describe('ResultDecorator', () => { timestamp: 100, targetPageTitle: 'test title', targetPageUrl: 'https://test_url', + framesSkipped: false, }; tagToLinkMapperMock @@ -240,6 +242,7 @@ describe('ResultDecorator', () => { timestamp: 100, targetPageTitle: 'test title', targetPageUrl: 'https://test_url', + framesSkipped: false, }; instanceStub.nodes = []; @@ -271,4 +274,43 @@ describe('ResultDecorator', () => { messageDecoratorMock.verifyAll(); }); }); + + describe('decorateResults: with incomplete results', () => { + it('should set framesSkipped', () => { + const incompleteInstance = { + id: 'frame-tested', + nodes: [], + description: null, + tags: ['tag2'], + }; + const guidanceLinkStub = {} as any; + const tagToLinkMapper = () => [guidanceLinkStub]; + const emptyResultsStub = { + passes: [], + violations: [], + inapplicable: [], + incomplete: [], + timestamp: 100, + targetPageTitle: 'test title', + targetPageUrl: 'https://test_url', + framesSkipped: true, + }; + messageDecoratorMock.setup(mdm => mdm.decorateResultWithMessages(It.isAny())); + ruleProcessorMock + .setup(m => m.suppressChecksByMessages(It.isAny(), It.isAny())) + .returns(result => { + return null; + }); + const testSubject = new ResultDecorator( + documentUtilsMock.object, + messageDecoratorMock.object, + getHelpUrlMock.object, + tagToLinkMapper, + ruleProcessorMock.object, + ); + nonEmptyResultStub.incomplete = [incompleteInstance]; + const decoratedResult = testSubject.decorateResults(nonEmptyResultStub); + expect(decoratedResult).toEqual(emptyResultsStub); + }); + }); }); diff --git a/src/tests/unit/tests/scanner/scan-parameter-generator.test.ts b/src/tests/unit/tests/scanner/scan-parameter-generator.test.ts index 6922b244332..0e34f03369a 100644 --- a/src/tests/unit/tests/scanner/scan-parameter-generator.test.ts +++ b/src/tests/unit/tests/scanner/scan-parameter-generator.test.ts @@ -17,6 +17,10 @@ describe('ScanParameterGenerator', () => { 'rule-b': { status: 'included', }, + 'rule-c': { + status: 'included-always', + reason: 'included always test', + }, }; const testObject = new ScanParameterGenerator(includedRulesStub); @@ -33,7 +37,7 @@ describe('ScanParameterGenerator', () => { restoreScroll: true, runOnly: { type: 'rule', - values: ['rule-a', 'rule-b'], + values: ['rule-a', 'rule-b', 'rule-c'], }, pingWaitTime: FRAME_COMMUNICATION_TIMEOUT_MS, }; @@ -48,7 +52,7 @@ describe('ScanParameterGenerator', () => { restoreScroll: true, runOnly: { type: 'rule', - values: ['rule-a', 'rule-b'], + values: ['rule-a', 'rule-b', 'rule-c'], }, pingWaitTime: FRAME_COMMUNICATION_TIMEOUT_MS, }; @@ -64,7 +68,7 @@ describe('ScanParameterGenerator', () => { restoreScroll: true, runOnly: { type: 'rule', - values: ['ruleA', 'ruleB'], + values: ['ruleA', 'ruleB', 'rule-c'], }, pingWaitTime: FRAME_COMMUNICATION_TIMEOUT_MS, };