From a42760605095255f560352bedcc8c88f4acc1db9 Mon Sep 17 00:00:00 2001
From: Chris LaFreniere <40371649+chlafreniere@users.noreply.github.com>
Date: Wed, 21 Oct 2020 16:55:41 -0700
Subject: [PATCH] WYSIWYG Span Style Fixes, Refactor, and Tests (#13011)

* Refactor into own class, add tests

* Add more tests

* Test fixes

* Test fix hopefully

* Tests D vs C drive
---
 .../browser/cellViews/textCell.component.ts   |  99 ++------------
 .../notebook/browser/htmlMarkdownConverter.ts | 110 ++++++++++++++++
 .../browser/htmlMarkdownConverter.test.ts     | 121 ++++++++++++++++++
 3 files changed, 240 insertions(+), 90 deletions(-)
 create mode 100644 src/sql/workbench/contrib/notebook/browser/htmlMarkdownConverter.ts
 create mode 100644 src/sql/workbench/contrib/notebook/test/browser/htmlMarkdownConverter.test.ts

diff --git a/src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.ts b/src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.ts
index 0a21a79afe..d34cc1b661 100644
--- a/src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.ts
+++ b/src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.ts
@@ -7,6 +7,7 @@ import 'vs/css!./media/markdown';
 import 'vs/css!./media/highlight';
 
 import { OnInit, Component, Input, Inject, forwardRef, ElementRef, ChangeDetectorRef, ViewChild, OnChanges, SimpleChange, HostListener, ViewChildren, QueryList } from '@angular/core';
+import * as Mark from 'mark.js';
 
 import { localize } from 'vs/nls';
 import { IWorkbenchThemeService } from 'vs/workbench/services/themes/common/workbenchThemeService';
@@ -15,9 +16,11 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti
 import { Emitter } from 'vs/base/common/event';
 import { URI } from 'vs/base/common/uri';
 import * as DOM from 'vs/base/browser/dom';
-
+import { IColorTheme } from 'vs/platform/theme/common/themeService';
+import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
 import { toDisposable } from 'vs/base/common/lifecycle';
 import { IMarkdownRenderResult } from 'vs/editor/contrib/markdown/markdownRenderer';
+
 import { NotebookMarkdownRenderer } from 'sql/workbench/contrib/notebook/browser/outputs/notebookMarkdown';
 import { CellView } from 'sql/workbench/contrib/notebook/browser/cellViews/interfaces';
 import { ICellModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces';
@@ -25,13 +28,8 @@ import { NotebookModel } from 'sql/workbench/services/notebook/browser/models/no
 import { ISanitizer, defaultSanitizer } from 'sql/workbench/services/notebook/browser/outputs/sanitizer';
 import { CodeComponent } from 'sql/workbench/contrib/notebook/browser/cellViews/code.component';
 import { NotebookRange, ICellEditorProvider, INotebookService } from 'sql/workbench/services/notebook/browser/notebookService';
-import { IColorTheme } from 'vs/platform/theme/common/themeService';
-import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
-import * as turndownPluginGfm from '../turndownPluginGfm';
-import TurndownService = require('turndown');
-import * as Mark from 'mark.js';
+import { HTMLMarkdownConverter } from 'sql/workbench/contrib/notebook/browser/htmlMarkdownConverter';
 import { NotebookInput } from 'sql/workbench/contrib/notebook/browser/models/notebookInput';
-import * as path from 'vs/base/common/path';
 
 export const TEXT_SELECTOR: string = 'text-cell-component';
 const USER_SELECT_CLASS = 'actionselect';
@@ -95,11 +93,11 @@ export class TextCellComponent extends CellView implements OnInit, OnChanges {
 	private _model: NotebookModel;
 	private _activeCellId: string;
 	private readonly _onDidClickLink = this._register(new Emitter<URI>());
-	public readonly onDidClickLink = this._onDidClickLink.event;
 	private markdownRenderer: NotebookMarkdownRenderer;
 	private markdownResult: IMarkdownRenderResult;
+	private _htmlMarkdownConverter: HTMLMarkdownConverter;
+	public readonly onDidClickLink = this._onDidClickLink.event;
 	public previewFeaturesEnabled: boolean = false;
-	private turndownService;
 	public doubleClickEditEnabled: boolean;
 
 	constructor(
@@ -111,7 +109,6 @@ export class TextCellComponent extends CellView implements OnInit, OnChanges {
 
 	) {
 		super();
-		this.setTurndownOptions();
 		this.markdownRenderer = this._instantiationService.createInstance(NotebookMarkdownRenderer);
 		this.doubleClickEditEnabled = this._configurationService.getValue('notebook.enableDoubleClickEdit');
 		this._register(toDisposable(() => {
@@ -162,6 +159,7 @@ export class TextCellComponent extends CellView implements OnInit, OnChanges {
 		this.updateTheme(this.themeService.getColorTheme());
 		this.setFocusAndScroll();
 		this.cellModel.isEditMode = false;
+		this._htmlMarkdownConverter = new HTMLMarkdownConverter(this.notebookUri);
 		this._register(this.cellModel.onOutputsChanged(e => {
 			this.updatePreview();
 		}));
@@ -248,7 +246,7 @@ export class TextCellComponent extends CellView implements OnInit, OnChanges {
 
 	private updateCellSource(): void {
 		let textOutputElement = <HTMLElement>this.output.nativeElement;
-		let newCellSource: string = this.turndownService.turndown(textOutputElement.innerHTML, { gfm: true });
+		let newCellSource: string = this._htmlMarkdownConverter.convert(textOutputElement.innerHTML);
 		this.cellModel.source = newCellSource;
 		this._changeRef.detectChanges();
 	}
@@ -437,68 +435,6 @@ export class TextCellComponent extends CellView implements OnInit, OnChanges {
 		return textOutput;
 	}
 
-	private setTurndownOptions() {
-		this.turndownService = new TurndownService({ 'emDelimiter': '_', 'bulletListMarker': '-', 'headingStyle': 'atx' });
-		this.turndownService.keep(['u', 'mark']);
-		this.turndownService.use(turndownPluginGfm.gfm);
-		this.turndownService.addRule('pre', {
-			filter: 'pre',
-			replacement: function (content, node) {
-				return '\n```\n' + node.textContent + '\n```\n';
-			}
-		});
-		this.turndownService.addRule('caption', {
-			filter: 'caption',
-			replacement: function (content, node) {
-				return `${node.outerHTML}
-				`;
-			}
-		});
-		this.turndownService.addRule('span', {
-			filter: function (node, options) {
-				return (
-					node.nodeName === 'MARK' ||
-					(node.nodeName === 'SPAN' &&
-						node.getAttribute('style') === 'background-color: yellow;')
-				);
-			},
-			replacement: function (content, node) {
-				if (node.nodeName === 'SPAN') {
-					return '<mark>' + node.textContent + '</mark>';
-				}
-				return node.textContent;
-			}
-		});
-		this.turndownService.addRule('img', {
-			filter: 'img',
-			replacement: (content, node) => {
-				if (node?.src) {
-					let imgPath = URI.parse(node.src);
-					const notebookFolder: string = this.notebookUri ? path.join(path.dirname(this.notebookUri.fsPath), path.sep) : '';
-					let relativePath = findPathRelativeToContent(notebookFolder, imgPath);
-					if (relativePath) {
-						return `![${node.alt}](${relativePath})`;
-					}
-				}
-				return `![${node.alt}](${node.src})`;
-			}
-		});
-		this.turndownService.addRule('a', {
-			filter: 'a',
-			replacement: (content, node) => {
-				//On Windows, if notebook is not trusted then the href attr is removed for all non-web URL links
-				// href contains either a hyperlink or a URI-encoded absolute path. (See resolveUrls method in notebookMarkdown.ts)
-				const notebookLink = node.href ? URI.parse(node.href) : URI.file(node.title);
-				const notebookFolder = this.notebookUri ? path.join(path.dirname(this.notebookUri.fsPath), path.sep) : '';
-				let relativePath = findPathRelativeToContent(notebookFolder, notebookLink);
-				if (relativePath) {
-					return `[${node.innerText}](${relativePath})`;
-				}
-				return `[${node.innerText}](${node.href})`;
-			}
-		});
-	}
-
 	// Enables edit mode on double clicking active cell
 	private enableActiveCellEditOnDoubleClick() {
 		if (!this.isEditMode && this.doubleClickEditEnabled) {
@@ -509,23 +445,6 @@ export class TextCellComponent extends CellView implements OnInit, OnChanges {
 	}
 }
 
-export function findPathRelativeToContent(notebookFolder: string, contentPath: URI | undefined): string {
-	if (notebookFolder) {
-		if (contentPath?.scheme === 'file') {
-			let relativePath = path.relative(notebookFolder, contentPath.fsPath);
-			//if path contains whitespaces then it's not identified as a link
-			relativePath = relativePath.replace(/\s/g, '%20');
-			if (relativePath.startsWith(path.join('..', path.sep) || path.join('.', path.sep))) {
-				return relativePath;
-			} else {
-				// if the relative path does not contain ./ at the beginning, we need to add it so it's recognized as a link
-				return `.${path.join(path.sep, relativePath)}`;
-			}
-		}
-	}
-	return '';
-}
-
 function preventDefaultAndExecCommand(e: KeyboardEvent, commandId: string) {
 	// use preventDefault() to avoid invoking the editor's select all
 	e.preventDefault();
diff --git a/src/sql/workbench/contrib/notebook/browser/htmlMarkdownConverter.ts b/src/sql/workbench/contrib/notebook/browser/htmlMarkdownConverter.ts
new file mode 100644
index 0000000000..91b253dbd2
--- /dev/null
+++ b/src/sql/workbench/contrib/notebook/browser/htmlMarkdownConverter.ts
@@ -0,0 +1,110 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the Source EULA. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import TurndownService = require('turndown');
+import { URI } from 'vs/base/common/uri';
+import * as path from 'vs/base/common/path';
+import * as turndownPluginGfm from 'sql/workbench/contrib/notebook/browser/turndownPluginGfm';
+
+export class HTMLMarkdownConverter {
+	private turndownService: TurndownService;
+
+	constructor(private notebookUri: URI) {
+		this.turndownService = new TurndownService({ 'emDelimiter': '_', 'bulletListMarker': '-', 'headingStyle': 'atx' });
+		this.setTurndownOptions();
+	}
+
+	public convert(html: string): string {
+		return this.turndownService.turndown(html, { gfm: true });
+	}
+
+	private setTurndownOptions() {
+		this.turndownService.keep(['u', 'mark']);
+		this.turndownService.use(turndownPluginGfm.gfm);
+		this.turndownService.addRule('pre', {
+			filter: 'pre',
+			replacement: function (content, node) {
+				return '\n```\n' + node.textContent + '\n```\n';
+			}
+		});
+		this.turndownService.addRule('caption', {
+			filter: 'caption',
+			replacement: function (content, node) {
+				return `${node.outerHTML}
+				`;
+			}
+		});
+		this.turndownService.addRule('span', {
+			filter: 'span',
+			replacement: function (content, node) {
+				let beginString = '';
+				let endString = '';
+				// TODO: handle other background colors and more styles
+				if (node?.style?.backgroundColor === 'yellow') {
+					beginString = '<mark>' + beginString;
+					endString += '</mark>';
+				}
+				if (node?.style?.fontWeight === 'bold') {
+					beginString = '**' + beginString;
+					endString += '**';
+				}
+				if (node?.style?.fontStyle === 'italic') {
+					beginString = '_' + beginString;
+					endString += '_';
+				}
+				if (node?.style?.textDecorationLine === 'underline') {
+					beginString = '<u>' + beginString;
+					endString += '</u>';
+				}
+				return beginString + node.textContent + endString;
+			}
+		});
+		this.turndownService.addRule('img', {
+			filter: 'img',
+			replacement: (content, node) => {
+				if (node?.src) {
+					let imgPath = URI.parse(node.src);
+					const notebookFolder: string = this.notebookUri ? path.join(path.dirname(this.notebookUri.fsPath), path.sep) : '';
+					let relativePath = findPathRelativeToContent(notebookFolder, imgPath);
+					if (relativePath) {
+						return `![${node.alt}](${relativePath})`;
+					}
+				}
+				return `![${node.alt}](${node.src})`;
+			}
+		});
+		this.turndownService.addRule('a', {
+			filter: 'a',
+			replacement: (content, node) => {
+				//On Windows, if notebook is not trusted then the href attr is removed for all non-web URL links
+				// href contains either a hyperlink or a URI-encoded absolute path. (See resolveUrls method in notebookMarkdown.ts)
+				const notebookLink = node.href ? URI.parse(node.href) : URI.file(node.title);
+				const notebookFolder = this.notebookUri ? path.join(path.dirname(this.notebookUri.fsPath), path.sep) : '';
+				let relativePath = findPathRelativeToContent(notebookFolder, notebookLink);
+				if (relativePath) {
+					return `[${node.innerText}](${relativePath})`;
+				}
+				return `[${node.innerText}](${node.href})`;
+			}
+		});
+	}
+}
+
+export function findPathRelativeToContent(notebookFolder: string, contentPath: URI | undefined): string {
+	if (notebookFolder) {
+		if (contentPath?.scheme === 'file') {
+			let relativePath = path.relative(notebookFolder, contentPath.fsPath);
+			//if path contains whitespaces then it's not identified as a link
+			relativePath = relativePath.replace(/\s/g, '%20');
+			if (relativePath.startsWith(path.join('..', path.sep) || path.join('.', path.sep))) {
+				return relativePath;
+			} else {
+				// if the relative path does not contain ./ at the beginning, we need to add it so it's recognized as a link
+				return `.${path.join(path.sep, relativePath)}`;
+			}
+		}
+	}
+	return '';
+}
diff --git a/src/sql/workbench/contrib/notebook/test/browser/htmlMarkdownConverter.test.ts b/src/sql/workbench/contrib/notebook/test/browser/htmlMarkdownConverter.test.ts
new file mode 100644
index 0000000000..02555d1517
--- /dev/null
+++ b/src/sql/workbench/contrib/notebook/test/browser/htmlMarkdownConverter.test.ts
@@ -0,0 +1,121 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the Source EULA. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import * as assert from 'assert';
+import { HTMLMarkdownConverter } from 'sql/workbench/contrib/notebook/browser/htmlMarkdownConverter';
+import * as path from 'vs/base/common/path';
+import { URI } from 'vs/base/common/uri';
+
+
+suite('HTML Markdown Converter', function (): void {
+	let htmlMarkdownConverter: HTMLMarkdownConverter;
+	let htmlString: string;
+
+	suiteSetup(() => {
+		htmlMarkdownConverter = new HTMLMarkdownConverter(URI.file('/tmp/notebook.ipynb'));
+		htmlString = '';
+	});
+
+	test('Should not alter HTML with no explicit elements', () => {
+		htmlString = 'Hello World 123';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), htmlString, 'No tags test failed');
+	});
+
+	test('Should keep <u> tag intact', () => {
+		htmlString = '<u>Hello test</u>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), htmlString, 'Basic underline test failed');
+		htmlString = 'Hello <u>test</u>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), htmlString, 'Partial underline test failed');
+		htmlString = '<p>Hello <u>test</u></p>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), 'Hello <u>test</u>', 'Partial underline paragraph test failed');
+		htmlString = '<h1>Hello <u>test</u></h1>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), '# Hello <u>test</u>', 'Partial underline h1 test failed');
+		htmlString = '<h2>Hello <u>test</u></h2>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), '## Hello <u>test</u>', 'Partial underline h2 test failed');
+		htmlString = '<h3>Hello <u>test</u></h3>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), '### Hello <u>test</u>', 'Partial underline h3 test failed');
+	});
+
+	test('Should keep <mark> tag intact', () => {
+		htmlString = '<mark>Hello test</mark>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), htmlString, 'Basic highlight test failed');
+		htmlString = 'Hello <mark>test</mark>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), htmlString, 'Partial highlight test failed');
+		htmlString = '<p>Hello <mark>test</mark></p>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), 'Hello <mark>test</mark>', 'Partial highlight paragraph test failed');
+		htmlString = '<h1>Hello <mark>test</mark></h1>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), '# Hello <mark>test</mark>', 'Partial highlight h1 test failed');
+		htmlString = '<h2>Hello <mark>test</mark></h2>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), '## Hello <mark>test</mark>', 'Partial highlight h2 test failed');
+		htmlString = '<h3>Hello <mark>test</mark></h3>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), '### Hello <mark>test</mark>', 'Partial highlight h3 test failed');
+	});
+
+	test('Should transform <pre> tag with ```', () => {
+		htmlString = '<pre>Hello test</pre>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), '```\nHello test\n```', 'Basic pre test failed');
+	});
+
+	test('Should transform <span> tag', () => {
+		htmlString = '<span>Hello test</span>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), 'Hello test', 'Basic span test failed');
+		htmlString = 'Yes<span style="background-color: yellow">Hello test</span>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), 'Yes<mark>Hello test</mark>', 'Basic yellow highlight span failed');
+		htmlString = 'Yes<span style="background-color:yellow">Hello test</span>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), 'Yes<mark>Hello test</mark>', 'Basic yellow highlight span no space failed');
+		htmlString = 'Yes<span style="font-weight: bold">Hello test</span>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), 'Yes**Hello test**', 'Basic bold span failed');
+		htmlString = 'Yes<span style="font-weight:bold">Hello test</span>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), 'Yes**Hello test**', 'Basic bold span no space failed');
+		htmlString = 'Yes<span style="font-style: italic">Hello test</span>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), 'Yes_Hello test_', 'Basic italic span failed');
+		htmlString = 'Yes<span style="font-style:italic">Hello test</span>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), 'Yes_Hello test_', 'Basic italic span no space failed');
+		htmlString = 'Yes<span style="text-decoration-line: underline">Hello test</span>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), 'Yes<u>Hello test</u>', 'Basic underline span failed');
+		htmlString = 'Yes<span style="text-decoration-line:underline">Hello test</span>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), 'Yes<u>Hello test</u>', 'Basic underline span no space failed');
+		htmlString = '<h1>Yes<span style="text-decoration-line:underline; font-style:italic; font-weight:bold; background-color: yellow">Hello test</span></h1>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), '# Yes<u>_**<mark>Hello test</mark>**_</u>', 'Compound elements span failed');
+	});
+
+	test('Should transform <img> tag', () => {
+		htmlString = '<img src="/tmp/stuff.png" alt="stuff">';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `![stuff](.${path.sep}stuff.png)`, 'Basic img test failed');
+		htmlString = '<img src="/tmp/stuff.png">';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `![](.${path.sep}stuff.png)`, 'Basic img test no alt failed');
+		htmlString = '<img src="/tmp/my stuff.png">';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `![](.${path.sep}my%20stuff.png)`, 'Basic img test no alt space filename failed');
+		htmlString = '<img src="/tmp/inner/stuff.png" alt="stuff">';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `![stuff](.${path.sep}inner${path.sep}stuff.png)`, 'Basic img test below folder failed');
+		htmlString = '<img src="/stuff.png" alt="stuff">';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `![stuff](..${path.sep}stuff.png)`, 'Basic img test above folder failed');
+		// htmlString = '<img src="e:\\some\\other\\path.png">';
+		// assert.equal(htmlMarkdownConverter.convert(htmlString), '![](e:\\some\\other\\path.png)', 'img test different drive failed');
+		htmlString = '<img src="https://www.microsoft.com/images/msft.png" alt="msft">';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), '![msft](https://www.microsoft.com/images/msft.png)', 'Basic https img test failed');
+		htmlString = '<img src="http://www.microsoft.com/images/msft.png" alt="msft">';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), '![msft](http://www.microsoft.com/images/msft.png)', 'Basic http img test failed');
+	});
+
+	test('Should transform <a> tag', () => {
+		htmlString = '<a href="/tmp/stuff.png">stuff</a>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `[stuff](.${path.sep}stuff.png)`, 'Basic link test failed');
+		htmlString = '<a href="/tmp/stuff.png"/>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `[](.${path.sep}stuff.png)`, 'Basic link test no label failed');
+		htmlString = '<a href="/tmp/my stuff.png"/>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `[](.${path.sep}my%20stuff.png)`, 'Basic link test no label space filename failed');
+		htmlString = '<a href="/stuff.png">stuff</a.';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `[stuff](..${path.sep}stuff.png)`, 'Basic link test above folder failed');
+		htmlString = '<a href="/tmp/inner/stuff.png">stuff</a>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `[stuff](.${path.sep}inner${path.sep}stuff.png)`, 'Basic link test below folder failed');
+		// htmlString = '<a href="e:\\some\\other\\path.png"/>';
+		// assert.equal(htmlMarkdownConverter.convert(htmlString), '[](e:\\some\\other\\path.png)', 'link test different drive failed');
+		htmlString = '<a href="https://www.microsoft.com/images/msft.png">msft</a>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), '[msft](https://www.microsoft.com/images/msft.png)', 'Basic https link test failed');
+		htmlString = '<a href="http://www.microsoft.com/images/msft.png">msft</a>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), '[msft](http://www.microsoft.com/images/msft.png)', 'Basic http link test failed');
+	});
+});