From e9495c1e60541c3c78832a963043769168927ede Mon Sep 17 00:00:00 2001 From: annkamsk Date: Tue, 25 Aug 2020 10:54:31 +0200 Subject: [PATCH] Use DOM API for creating HTML elements instead of string concatenation --- src/vs/base/browser/dom.ts | 12 + .../issue/issueReporterMain.ts | 221 ++++++++++-------- 2 files changed, 137 insertions(+), 96 deletions(-) diff --git a/src/vs/base/browser/dom.ts b/src/vs/base/browser/dom.ts index 29490d6fc1e..0e254009357 100644 --- a/src/vs/base/browser/dom.ts +++ b/src/vs/base/browser/dom.ts @@ -1009,6 +1009,18 @@ export function prepend(parent: HTMLElement, child: T): T { const SELECTOR_REGEX = /([\w\-]+)?(#([\w\-]+))?((\.([\w\-]+))*)/; +export function reset(parent: HTMLElement, ...children: Array) { + parent.innerText = ''; + coalesce(children) + .forEach(child => { + if (child instanceof Node) { + parent.appendChild(child); + } else { + parent.appendChild(document.createTextNode(child as string)); + } + }); +} + export enum Namespace { HTML = 'http://www.w3.org/1999/xhtml', SVG = 'http://www.w3.org/2000/svg' diff --git a/src/vs/code/electron-sandbox/issue/issueReporterMain.ts b/src/vs/code/electron-sandbox/issue/issueReporterMain.ts index fd820cf9f58..cc33ad9c397 100644 --- a/src/vs/code/electron-sandbox/issue/issueReporterMain.ts +++ b/src/vs/code/electron-sandbox/issue/issueReporterMain.ts @@ -8,7 +8,7 @@ import 'vs/base/browser/ui/codicons/codiconStyles'; // make sure codicon css is import { ElectronService, IElectronService } from 'vs/platform/electron/electron-sandbox/electron'; import { ipcRenderer, process } from 'vs/base/parts/sandbox/electron-sandbox/globals'; import { applyZoom, zoomIn, zoomOut } from 'vs/platform/windows/electron-sandbox/window'; -import { $, windowOpenNoOpener, addClass } from 'vs/base/browser/dom'; +import { $, reset, windowOpenNoOpener, addClass } from 'vs/base/browser/dom'; import { Button } from 'vs/base/browser/ui/button/button'; import { CodiconLabel } from 'vs/base/browser/ui/codicons/codiconLabel'; import * as collections from 'vs/base/common/collections'; @@ -277,33 +277,25 @@ export class IssueReporter extends Disposable { } private updateSettingsSearchDetails(data: ISettingsSearchIssueReporterData): void { - const target = document.querySelector('.block-settingsSearchResults .block-info'); + const target = document.querySelector('.block-settingsSearchResults .block-info'); if (target) { - const details = ` -
-
Query: "${data.query}"
-
Literal match count: ${data.filterResultCount}
-
- `; + const queryDiv = $('div', undefined, `Query: "${data.query}"` as string); + const countDiv = $('div', undefined, `Literal match count: ${data.filterResultCount}` as string); + const detailsDiv = $('.block-settingsSearchResults-details', undefined, queryDiv, countDiv); - let table = ` - - Setting - Extension - Score - `; - - data.actualSearchResults - .forEach(setting => { - table += ` - - ${setting.key} - ${setting.extensionId} - ${String(setting.score).slice(0, 5)} - `; - }); - - target.innerHTML = `${details}${table}
`; + const table = $('table', undefined, + $('tr', undefined, + $('th', undefined, 'Setting'), + $('th', undefined, 'Extension'), + $('th', undefined, 'Score'), + ), + ...data.actualSearchResults.map(setting => $('tr', undefined, + $('td', undefined, setting.key), + $('td', undefined, setting.extensionId), + $('td', undefined, String(setting.score).slice(0, 5)), + )) + ); + reset(target, detailsDiv, table); } } @@ -654,9 +646,9 @@ export class IssueReporter extends Disposable { issueState.appendChild(issueIcon); issueState.appendChild(issueStateLabel); - item = $('div.issue', {}, issueState, link); + item = $('div.issue', undefined, issueState, link); } else { - item = $('div.issue', {}, link); + item = $('div.issue', undefined, link); } issues.appendChild(item); @@ -672,19 +664,19 @@ export class IssueReporter extends Disposable { } private setUpTypes(): void { - const makeOption = (issueType: IssueType, description: string) => ``; + const makeOption = (issueType: IssueType, description: string) => $('option', { 'value': issueType.valueOf() }, escape(description)); const typeSelect = this.getElementById('issue-type')! as HTMLSelectElement; const { issueType } = this.issueReporterModel.getData(); if (issueType === IssueType.SettingsSearchIssue) { - typeSelect.innerHTML = makeOption(IssueType.SettingsSearchIssue, localize('settingsSearchIssue', "Settings Search Issue")); + reset(typeSelect, makeOption(IssueType.SettingsSearchIssue, localize('settingsSearchIssue', "Settings Search Issue"))); typeSelect.disabled = true; } else { - typeSelect.innerHTML = [ + reset(typeSelect, makeOption(IssueType.Bug, localize('bugReporter', "Bug Report")), makeOption(IssueType.FeatureRequest, localize('featureRequest', "Feature Request")), makeOption(IssueType.PerformanceIssue, localize('performanceIssue', "Performance Issue")) - ].join('\n'); + ); } typeSelect.value = issueType.toString(); @@ -774,9 +766,8 @@ export class IssueReporter extends Disposable { } else { show(extensionsBlock); } - - descriptionTitle.innerHTML = `${localize('stepsToReproduce', "Steps to Reproduce")} *`; - descriptionSubtitle.innerHTML = localize('bugDescription', "Share the steps needed to reliably reproduce the problem. Please include actual and expected results. We support GitHub-flavored Markdown. You will be able to edit your issue and add screenshots when we preview it on GitHub."); + reset(descriptionTitle, localize('stepsToReproduce', "Steps to Reproduce"), $('span.required-input', undefined, '*')); + reset(descriptionSubtitle, localize('bugDescription', "Share the steps needed to reliably reproduce the problem. Please include actual and expected results. We support GitHub-flavored Markdown. You will be able to edit your issue and add screenshots when we preview it on GitHub.")); } else if (issueType === IssueType.PerformanceIssue) { show(blockContainer); show(systemBlock); @@ -790,11 +781,11 @@ export class IssueReporter extends Disposable { show(extensionsBlock); } - descriptionTitle.innerHTML = `${localize('stepsToReproduce', "Steps to Reproduce")} *`; - descriptionSubtitle.innerHTML = localize('performanceIssueDesciption', "When did this performance issue happen? Does it occur on startup or after a specific series of actions? We support GitHub-flavored Markdown. You will be able to edit your issue and add screenshots when we preview it on GitHub."); + reset(descriptionTitle, localize('stepsToReproduce', "Steps to Reproduce"), $('span.required-input', undefined, '*')); + reset(descriptionSubtitle, localize('performanceIssueDesciption', "When did this performance issue happen? Does it occur on startup or after a specific series of actions? We support GitHub-flavored Markdown. You will be able to edit your issue and add screenshots when we preview it on GitHub.")); } else if (issueType === IssueType.FeatureRequest) { - descriptionTitle.innerHTML = `${localize('description', "Description")} *`; - descriptionSubtitle.innerHTML = localize('featureRequestDescription', "Please describe the feature you would like to see. We support GitHub-flavored Markdown. You will be able to edit your issue and add screenshots when we preview it on GitHub."); + reset(descriptionTitle, localize('description', "Description"), $('span.required-input', undefined, '*')); + reset(descriptionSubtitle, localize('featureRequestDescription', "Please describe the feature you would like to see. We support GitHub-flavored Markdown. You will be able to edit your issue and add screenshots when we preview it on GitHub.")); show(problemSource); if (fileOnExtension) { @@ -805,8 +796,8 @@ export class IssueReporter extends Disposable { show(searchedExtensionsBlock); show(settingsSearchResultsBlock); - descriptionTitle.innerHTML = `${localize('expectedResults', "Expected Results")} *`; - descriptionSubtitle.innerHTML = localize('settingsSearchResultsDescription', "Please list the results that you were expecting to see when you searched with this query. We support GitHub-flavored Markdown. You will be able to edit your issue and add screenshots when we preview it on GitHub."); + reset(descriptionTitle, localize('expectedResults', "Expected Results"), $('span.required-input', undefined, '*')); + reset(descriptionSubtitle, localize('settingsSearchResultsDescription', "Please list the results that you were expecting to see when you searched with this query. We support GitHub-flavored Markdown. You will be able to edit your issue and add screenshots when we preview it on GitHub.")); } } @@ -928,42 +919,82 @@ export class IssueReporter extends Disposable { } private updateSystemInfo(state: IssueReporterModelData) { - const target = document.querySelector('.block-system .block-info'); + const target = document.querySelector('.block-system .block-info'); + if (target) { const systemInfo = state.systemInfo!; - let renderedData = ` - - - - - - - - -
CPUs${systemInfo.cpus}
GPU Status${Object.keys(systemInfo.gpuStatus).map(key => `${key}: ${systemInfo.gpuStatus[key]}`).join('
')}
Load (avg)${systemInfo.load}
Memory (System)${systemInfo.memory}
Process Argv${systemInfo.processArgs}
Screen Reader${systemInfo.screenReader}
VM${systemInfo.vmHint}
`; + const renderedDataTable = $('table', undefined, + $('tr', undefined, + $('td', undefined, 'CPUs'), + $('td', undefined, systemInfo.cpus || ''), + ), + $('tr', undefined, + $('td', undefined, 'GPU Status' as string), + $('td', undefined, Object.keys(systemInfo.gpuStatus).map(key => `${key}: ${systemInfo.gpuStatus[key]}`).join('\n')), + ), + $('tr', undefined, + $('td', undefined, 'Load (avg)' as string), + $('td', undefined, systemInfo.load || ''), + ), + $('tr', undefined, + $('td', undefined, 'Memory (System)' as string), + $('td', undefined, systemInfo.memory), + ), + $('tr', undefined, + $('td', undefined, 'Process Argv' as string), + $('td', undefined, systemInfo.processArgs), + ), + $('tr', undefined, + $('td', undefined, 'Screen Reader' as string), + $('td', undefined, systemInfo.screenReader), + ), + $('tr', undefined, + $('td', undefined, 'VM'), + $('td', undefined, systemInfo.vmHint), + ), + ); + reset(target, renderedDataTable); systemInfo.remoteData.forEach(remote => { + target.appendChild($('hr')); if (isRemoteDiagnosticError(remote)) { - renderedData += ` -
- - - -
Remote${remote.hostName}
${remote.errorMessage}
`; + const remoteDataTable = $('table', undefined, + $('tr', undefined, + $('td', undefined, 'Remote'), + $('td', undefined, remote.hostName) + ), + $('tr', undefined, + $('td', undefined, ''), + $('td', undefined, remote.errorMessage) + ) + ); + target.appendChild(remoteDataTable); } else { - renderedData += ` -
- - - - - - -
Remote${remote.hostName}
OS${remote.machineInfo.os}
CPUs${remote.machineInfo.cpus}
Memory (System)${remote.machineInfo.memory}
VM${remote.machineInfo.vmHint}
`; + const remoteDataTable = $('table', undefined, + $('tr', undefined, + $('td', undefined, 'Remote'), + $('td', undefined, remote.hostName) + ), + $('tr', undefined, + $('td', undefined, 'OS'), + $('td', undefined, remote.machineInfo.os) + ), + $('tr', undefined, + $('td', undefined, 'CPUs'), + $('td', undefined, remote.machineInfo.cpus || '') + ), + $('tr', undefined, + $('td', undefined, 'Memory (System)' as string), + $('td', undefined, remote.machineInfo.memory) + ), + $('tr', undefined, + $('td', undefined, 'VM'), + $('td', undefined, remote.machineInfo.vmHint) + ), + ); + target.appendChild(remoteDataTable); } }); - - target.innerHTML = renderedData; } } @@ -995,15 +1026,18 @@ export class IssueReporter extends Disposable { return 0; }); - const makeOption = (extension: IOption, selectedExtension?: IssueReporterExtensionData) => { + const makeOption = (extension: IOption, selectedExtension?: IssueReporterExtensionData): HTMLOptionElement => { const selected = selectedExtension && extension.id === selectedExtension.id; - return ``; + return $('option', { + 'value': extension.id, + 'selected': selected || '' + }, extension.name); }; const extensionsSelector = this.getElementById('extension-selector'); if (extensionsSelector) { const { selectedExtension } = this.issueReporterModel.getData(); - extensionsSelector.innerHTML = '' + extensionOptions.map(extension => makeOption(extension, selectedExtension)).join('\n'); + reset(extensionsSelector, $('option'), ...extensionOptions.map(extension => makeOption(extension, selectedExtension))); this.addEventListener('extension-selector', 'change', (e: Event) => { const selectedExtensionId = (e.target).value; @@ -1071,9 +1105,9 @@ export class IssueReporter extends Disposable { } private updateProcessInfo(state: IssueReporterModelData) { - const target = document.querySelector('.block-process .block-info'); + const target = document.querySelector('.block-process .block-info') as HTMLElement; if (target) { - target.innerHTML = `${state.processInfo}`; + reset(target, $('code', undefined, state.processInfo)); } } @@ -1085,7 +1119,7 @@ export class IssueReporter extends Disposable { const target = document.querySelector('.block-extensions .block-info'); if (target) { if (this.configuration.disableExtensions) { - target.innerHTML = localize('disabledExtensions', "Extensions are disabled"); + reset(target, localize('disabledExtensions', "Extensions are disabled")); return; } @@ -1097,8 +1131,7 @@ export class IssueReporter extends Disposable { return; } - const table = this.getExtensionTableHtml(extensions); - target.innerHTML = `${table}
${themeExclusionStr}`; + reset(target, this.getExtensionTableHtml(extensions), document.createTextNode(themeExclusionStr)); } } @@ -1111,28 +1144,24 @@ export class IssueReporter extends Disposable { } const table = this.getExtensionTableHtml(extensions); - target.innerHTML = `${table}
`; + target.innerText = ''; + target.appendChild(table); } } - private getExtensionTableHtml(extensions: IssueReporterExtensionData[]): string { - let table = ` - - Extension - Author (truncated) - Version - `; - - table += extensions.map(extension => { - return ` - - ${extension.name} - ${extension.publisher.substr(0, 3)} - ${extension.version} - `; - }).join(''); - - return table; + private getExtensionTableHtml(extensions: IssueReporterExtensionData[]): HTMLTableElement { + return $('table', undefined, + $('tr', undefined, + $('th', undefined, 'Extension'), + $('th', undefined, 'Author (truncated)' as string), + $('th', undefined, 'Version'), + ), + ...extensions.map(extension => $('tr', undefined, + $('td', undefined, extension.name), + $('td', undefined, extension.publisher.substr(0, 3)), + $('td', undefined, extension.version), + )) + ); } private openLink(event: MouseEvent): void {