chore(null): strict null fixes for dialog-renderer deps (#6223)

#### Details

This PR adds a bunch of `injected/visualization` files to the
strictNullCheck set. To do this, I introduced an interface seam for
`DialogRenderer` (since it has dependencies on lots of non-null-clean
components) and then did two rounds of `yarn null:find`/`yarn
null:autoadd`.

I also included a few fixes for dialog-renderer-impl that I started
making before deciding to introduce the interface seam. It is
intentionally omitted from the strictNullChecks config since it still
has non-clean dependencies.

##### Motivation

See #2869

##### Context

n/a

#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a"
in the checkbox -->
- [x] Addresses an existing issue: #2869
- [x] Ran `yarn null:autoadd`
- [x] Ran `yarn fastpass`
- [x] Added/updated relevant unit test(s) (and ran `yarn test`)
- [x] Verified code coverage for the changes made. Check coverage report
at: `<rootDir>/test-results/unit/coverage`
- [x] PR title *AND* final merge commit title both start with a semantic
tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See
`CONTRIBUTING.md`.
- [n/a] (UI changes only) Added screenshots/GIFs to description above
- [n/a] (UI changes only) Verified usability with NVDA/JAWS

src/injected/analyzers/tab-stops-analyzer.ts

@@ -40,7 +40,7 @@ export class TabStopsAnalyzer extends BaseAnalyzer {
         this.debouncedProcessTabEvents = this.debounceImpl(this.processTabEvents, 50);
         this.tabStopListenerRunner.topWindowCallback = (tabEvent: TabStopEvent) => {
             this.pendingTabbedElements.push(tabEvent);
-            this.debouncedProcessTabEvents();
+            this.debouncedProcessTabEvents!();
         };
         await this.tabStopListenerRunner.start();
 

src/injected/dialog-renderer.tsx → src/injected/dialog-renderer-impl.tsx

@@ -15,6 +15,7 @@ import {
     HtmlElementAxeResults,
 } from 'common/types/store-data/visualization-scan-result-data';
 import { WindowUtils } from 'common/window-utils';
+import { DialogRenderer } from 'injected/dialog-renderer';
 import {
     CommandMessage,
     CommandMessageResponse,
@@ -36,13 +37,7 @@ import {
 } from './layered-details-dialog-component';
 import { MainWindowContext } from './main-window-context';
 
-export interface DetailsDialogWindowMessage {
-    data: HtmlElementAxeResults;
-}
-
-export type RenderDialog = (data: HtmlElementAxeResults) => void;
-
-export class DialogRenderer {
+export class DialogRendererImpl implements DialogRenderer {
     private static readonly renderDetailsDialogCommand = 'insights.detailsDialog';
 
     constructor(
@@ -56,16 +51,16 @@ export class DialogRenderer {
         private readonly getRTLFunc: typeof getRTL,
         private readonly detailsDialogHandler: DetailsDialogHandler,
     ) {
-        if (this.isInMainWindow()) {
+        if (this.windowUtils.isTopWindow()) {
             this.frameMessenger.addMessageListener(
-                DialogRenderer.renderDetailsDialogCommand,
+                DialogRendererImpl.renderDetailsDialogCommand,
                 this.processRequest,
             );
         }
     }
 
     public render = async (data: HtmlElementAxeResults): Promise<CommandMessageResponse | null> => {
-        if (this.isInMainWindow()) {
+        if (this.windowUtils.isTopWindow()) {
             const mainWindowContext = MainWindowContext.fromWindow(this.windowUtils.getWindow());
             mainWindowContext.getTargetPageActionMessageCreator().openIssuesDialog();
 
@@ -121,14 +116,21 @@ export class DialogRenderer {
             );
             return null;
         } else {
+            const topWindow = this.windowUtils.getTopWindow();
+            if (topWindow == null) {
+                // This is only expected if this frame's window has been detached from the
+                // corresponding browser navigable entity, eg because it is in the process of
+                // unloading. Particularly, topWindow will *not* be null merely if the top window
+                // is across an origin boundary. If we're in this state, we intentionally abandon
+                // rendering.
+                return null;
+            }
+
             const message: CommandMessage = {
-                command: DialogRenderer.renderDetailsDialogCommand,
+                command: DialogRendererImpl.renderDetailsDialogCommand,
                 payload: { data: data },
             };
-            return await this.frameMessenger.sendMessageToWindow(
-                this.windowUtils.getTopWindow(),
-                message,
-            );
+            return await this.frameMessenger.sendMessageToWindow(topWindow, message);
         }
     };
 
@@ -146,7 +148,11 @@ export class DialogRenderer {
 
         const dialogContainer = this.dom.createElement('div');
         dialogContainer.setAttribute('class', 'insights-dialog-container');
-        this.dom.querySelector(`#${rootContainerId}`).appendChild(dialogContainer);
+        const rootContainer = this.dom.querySelector(`#${rootContainerId}`);
+        if (rootContainer == null) {
+            throw new Error(`Could not find #${rootContainerId} element to inject dialog into`);
+        }
+        rootContainer.appendChild(dialogContainer);
         return dialogContainer;
     }
 
@@ -163,8 +169,4 @@ export class DialogRenderer {
     private getElementSelector(data: HtmlElementAxeResults): string {
         return TargetHelper.getSelectorFromTarget(data.target);
     }
-
-    private isInMainWindow(): boolean {
-        return this.windowUtils.getTopWindow() === this.windowUtils.getWindow();
-    }
 }

src/injected/dialog-renderer.ts

@@ -0,0 +1,14 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+import { HtmlElementAxeResults } from 'common/types/store-data/visualization-scan-result-data';
+import { CommandMessageResponse } from 'injected/frameCommunicators/respondable-command-message-communicator';
+
+export interface DetailsDialogWindowMessage {
+    data: HtmlElementAxeResults;
+}
+
+export type RenderDialog = (data: HtmlElementAxeResults) => void;
+
+export interface DialogRenderer {
+    render(data: HtmlElementAxeResults): Promise<CommandMessageResponse | null>;
+}

src/injected/visualization/base-drawer.ts

@@ -17,7 +17,7 @@ export abstract class BaseDrawer implements Drawer {
     public static recalculationTimeout = 16;
     protected dialogRenderer: DialogRenderer;
     protected windowUtils: WindowUtils;
-    protected containerElement: HTMLElement;
+    protected containerElement: HTMLElement | null;
     protected drawerUtils: DrawerUtils;
 
     private shadowUtils: ShadowUtils;
@@ -105,6 +105,10 @@ export abstract class BaseDrawer implements Drawer {
     };
 
     protected applyContainerClass(): void {
+        if (this.containerElement == null) {
+            throw new Error('applyContainerClass invoked in illegal state (no containerElement)');
+        }
+
         this.containerElement.setAttribute(
             'class',
             `insights-container insights-highlight-container ${this.containerClass}`,
@@ -127,6 +131,10 @@ export abstract class BaseDrawer implements Drawer {
     }
 
     private attachContainerToDom(): void {
+        if (this.containerElement == null) {
+            throw new Error('attachContainerToDom invoked in illegal state (no containerElement)');
+        }
+
         this.shadowUtils.getShadowContainer().appendChild(this.containerElement);
     }
 }

src/injected/visualization/failure-instance-formatter.ts

@@ -17,13 +17,15 @@ export abstract class FailureInstanceFormatter implements Formatter {
 
     public abstract getDrawerConfiguration(element: Node, data: AxeResultsWithFrameLevel): any;
 
-    public abstract getDialogRenderer(): DialogRenderer;
+    public abstract getDialogRenderer(): DialogRenderer | null;
 
-    protected getFailureBoxConfig(data: AssessmentVisualizationInstance): FailureBoxConfig {
+    protected getFailureBoxConfig(
+        data: AssessmentVisualizationInstance,
+    ): FailureBoxConfig | undefined {
         if (data && data.isFailure) {
             return FailureInstanceFormatter.failureBoxConfig;
         }
 
-        return null;
+        return undefined;
     }
 }

src/injected/visualization/formatter.ts

@@ -81,5 +81,5 @@ export interface Formatter {
         element: Node,
         data: AxeResultsWithFrameLevel,
     ): DrawerConfiguration | SVGDrawerConfiguration | SingleTargetDrawerConfiguration;
-    getDialogRenderer(): DialogRenderer;
+    getDialogRenderer(): DialogRenderer | null;
 }

src/injected/visualization/frame-formatter.ts

@@ -30,7 +30,7 @@ export class FrameFormatter extends FailureInstanceFormatter {
         },
     };
 
-    public getDialogRenderer(): DialogRenderer {
+    public getDialogRenderer(): DialogRenderer | null {
         return null;
     }
 

src/injected/visualization/heading-formatter.ts

@@ -54,17 +54,15 @@ export class HeadingFormatter extends FailureInstanceFormatter {
         },
     };
 
-    public getDialogRenderer(): DialogRenderer {
+    public getDialogRenderer(): DialogRenderer | null {
         return null;
     }
 
     public getDrawerConfiguration(
         element: HTMLElement,
         data: AssessmentVisualizationInstance,
     ): DrawerConfiguration {
-        const level = this.getAriaLevel(element)
-            ? this.getAriaLevel(element)
-            : this.getHTagLevel(element);
+        const level = this.getAriaLevel(element) ?? this.getHTagLevel(element);
         const text = (this.isHTag(element) ? 'H' : 'h') + level;
         const style = HeadingFormatter.headingStyles[level] || HeadingFormatter.headingStyles.blank;
 
@@ -111,12 +109,12 @@ export class HeadingFormatter extends FailureInstanceFormatter {
         return headingLevel ? headingLevel[1] : '-';
     }
 
-    private getAriaLevel(element: HTMLElement): string {
+    private getAriaLevel(element: HTMLElement): string | null {
         const attr = element.attributes.getNamedItem('aria-level');
         return attr ? attr.textContent : null;
     }
 
-    private getAttribute(element: HTMLElement, attrName: string): string {
+    private getAttribute(element: HTMLElement, attrName: string): string | null {
         const attr = element.attributes.getNamedItem(attrName);
         return attr ? attr.textContent : null;
     }

src/injected/visualization/issues-formatter.ts

@@ -3,6 +3,7 @@
 import { getRTL } from '@fluentui/utilities';
 import { NavigatorUtils } from 'common/navigator-utils';
 import { HtmlElementAxeResults } from 'common/types/store-data/visualization-scan-result-data';
+import { DialogRendererImpl } from 'injected/dialog-renderer-impl';
 import { SingleFrameMessenger } from 'injected/frameCommunicators/single-frame-messenger';
 import * as ReactDOM from 'react-dom';
 
@@ -28,7 +29,7 @@ export class IssuesFormatter implements Formatter {
         getRTLFunc: typeof getRTL,
         detailsDialogHandler: DetailsDialogHandler,
     ) {
-        this.dialogRenderer = new DialogRenderer(
+        this.dialogRenderer = new DialogRendererImpl(
             document,
             ReactDOM.render,
             frameMessenger,

src/injected/visualization/single-target-drawer.ts

@@ -9,7 +9,7 @@ import { SingleTargetFormatter } from './single-target-formatter';
 export class SingleTargetDrawer implements Drawer {
     protected isEnabled = false;
     protected drawerUtils: DrawerUtils;
-    private target: HTMLElement;
+    private target: HTMLElement | null;
     private formatter: SingleTargetFormatter;
 
     constructor(drawerUtils: DrawerUtils, formatter: SingleTargetFormatter) {
@@ -48,7 +48,7 @@ export class SingleTargetDrawer implements Drawer {
     private getFirstElementTarget(
         document: Document,
         elementResults: HtmlElementAxeResults[],
-    ): HTMLElement {
+    ): HTMLElement | null {
         if (!elementResults[0]) {
             return null;
         }

src/injected/visualization/single-target-formatter.ts

@@ -13,7 +13,7 @@ export class SingleTargetFormatter implements Formatter {
         return config;
     }
 
-    public getDialogRenderer(): DialogRenderer {
-        return;
+    public getDialogRenderer(): DialogRenderer | null {
+        return null;
     }
 }

src/injected/visualization/svg-shape-factory.ts

@@ -101,6 +101,12 @@ export class SVGShapeFactory {
         return text;
     }
 
+    private setOptionalAttribute(element: Element, attributeName: string, attributeValue?: string) {
+        if (attributeValue != null) {
+            element.setAttributeNS(null, attributeName, attributeValue);
+        }
+    }
+
     public createFailureLabel(center: Point, failureBoxConfig: FailureBoxConfig): Element {
         const myDocument = this.drawerUtils.getDocumentElement();
 
@@ -109,19 +115,20 @@ export class SVGShapeFactory {
         box.classList.add('failure-label');
         box.setAttributeNS(null, 'x', (center.x + 10).toString());
         box.setAttributeNS(null, 'y', (center.y - 20).toString());
-        box.setAttributeNS(null, 'width', failureBoxConfig.boxWidth);
-        box.setAttributeNS(null, 'height', failureBoxConfig.boxWidth);
         box.setAttributeNS(null, 'fill', failureBoxConfig.background);
-        box.setAttributeNS(null, 'rx', failureBoxConfig.cornerRadius);
+
+        this.setOptionalAttribute(box, 'width', failureBoxConfig.boxWidth);
+        this.setOptionalAttribute(box, 'height', failureBoxConfig.boxWidth);
+        this.setOptionalAttribute(box, 'rx', failureBoxConfig.cornerRadius);
 
         const text = myDocument.createElementNS(SVGNamespaceUrl, 'text');
         text.classList.add('insights-highlight-text');
         text.classList.add('failure-label');
         text.setAttributeNS(null, 'x', (center.x + 13.5).toString());
         text.setAttributeNS(null, 'y', (center.y - 12).toString());
         text.setAttributeNS(null, 'fill', failureBoxConfig.fontColor);
-        text.setAttributeNS(null, 'font-size', failureBoxConfig.fontSize);
-        text.setAttributeNS(null, 'font-weight', failureBoxConfig.fontWeight);
+        this.setOptionalAttribute(text, 'font-size', failureBoxConfig.fontSize);
+        this.setOptionalAttribute(text, 'font-weight', failureBoxConfig.fontWeight);
         text.innerHTML = failureBoxConfig.text;
 
         const group = myDocument.createElementNS(SVGNamespaceUrl, 'g');

src/injected/visualization/table-headers-formatter.ts

@@ -14,7 +14,7 @@ export class TableHeadersAttributeFormatter extends FailureInstanceFormatter {
         super();
     }
 
-    public getDialogRenderer(): DialogRenderer {
+    public getDialogRenderer(): DialogRenderer | null {
         return null;
     }
 

src/tests/unit/tests/injected/dialog-renderer.test.tsx → src/tests/unit/tests/injected/dialog-renderer-impl.test.tsx

@@ -18,6 +18,7 @@ import { WindowUtils } from 'common/window-utils';
 import { rootContainerId } from 'injected/constants';
 import { DetailsDialogHandler } from 'injected/details-dialog-handler';
 import { DialogRenderer } from 'injected/dialog-renderer';
+import { DialogRendererImpl } from 'injected/dialog-renderer-impl';
 import {
     CommandMessage,
     CommandMessageResponse,
@@ -31,7 +32,7 @@ import * as ReactDOM from 'react-dom';
 import { IMock, It, Mock, MockBehavior, Times } from 'typemoq';
 import { DictionaryStringTo } from 'types/common-types';
 
-describe(DialogRenderer, () => {
+describe(DialogRendererImpl, () => {
     let htmlElementUtilsMock: IMock<HTMLElementUtils>;
     let windowUtilsMock: IMock<WindowUtils>;
     let navigatorUtilsMock: IMock<NavigatorUtils>;
@@ -267,8 +268,8 @@ describe(DialogRenderer, () => {
             .setup(wum => wum.getTopWindow())
             .returns(() => {
                 return windowStub;
-            })
-            .verifiable(Times.atLeastOnce());
+            });
+        windowUtilsMock.setup(wum => wum.isTopWindow()).returns(() => true);
         windowUtilsMock.setup(wum => wum.getPlatform()).returns(() => 'Win32');
         frameMessenger
             .setup(fm => fm.addMessageListener(It.isValue('insights.detailsDialog'), It.isAny()))
@@ -293,10 +294,8 @@ describe(DialogRenderer, () => {
             id: 'this is a different window than windowStub',
         } as unknown as Window;
 
-        windowUtilsMock
-            .setup(wum => wum.getTopWindow())
-            .returns(() => topWindowStub)
-            .verifiable(Times.atLeastOnce());
+        windowUtilsMock.setup(wum => wum.getTopWindow()).returns(() => topWindowStub);
+        windowUtilsMock.setup(wum => wum.isTopWindow()).returns(() => false);
 
         frameMessenger
             .setup(fm => fm.addMessageListener(It.isValue(commandMessage.command), It.isAny()))
@@ -331,7 +330,7 @@ describe(DialogRenderer, () => {
     }
 
     function createDialogRenderer(): DialogRenderer {
-        return new DialogRenderer(
+        return new DialogRendererImpl(
             domMock.object,
             renderMock.object,
             frameMessenger.object,

src/tests/unit/tests/injected/visualization/__snapshots__/table-headers-formatter.test.ts.snap

@@ -2,7 +2,7 @@
 
 exports[`TableHeadersAttributeFormatter getDrawerConfiguration - td headers=headers role=null id=id 1`] = `
 {
-  "failureBoxConfig": null,
+  "failureBoxConfig": undefined,
   "outlineColor": "#6600CC",
   "outlineStyle": "solid",
   "showVisualization": true,
@@ -17,7 +17,7 @@ exports[`TableHeadersAttributeFormatter getDrawerConfiguration - td headers=head
 
 exports[`TableHeadersAttributeFormatter getDrawerConfiguration - td headers=headers role=null id=null 1`] = `
 {
-  "failureBoxConfig": null,
+  "failureBoxConfig": undefined,
   "outlineColor": "#6600CC",
   "outlineStyle": "solid",
   "showVisualization": true,
@@ -32,7 +32,7 @@ exports[`TableHeadersAttributeFormatter getDrawerConfiguration - td headers=head
 
 exports[`TableHeadersAttributeFormatter getDrawerConfiguration - th headers=headers role=null id=id 1`] = `
 {
-  "failureBoxConfig": null,
+  "failureBoxConfig": undefined,
   "outlineColor": "#0066CC",
   "outlineStyle": "solid",
   "showVisualization": true,
@@ -47,7 +47,7 @@ exports[`TableHeadersAttributeFormatter getDrawerConfiguration - th headers=head
 
 exports[`TableHeadersAttributeFormatter getDrawerConfiguration - th headers=null role=null id=id 1`] = `
 {
-  "failureBoxConfig": null,
+  "failureBoxConfig": undefined,
   "outlineColor": "#0066CC",
   "outlineStyle": "solid",
   "showVisualization": true,