From 1760af13d1d5e3653e7b45ad17a70aba494eb1d9 Mon Sep 17 00:00:00 2001
From: Maddy <12754347+MaddyDev@users.noreply.github.com>
Date: Thu, 21 Nov 2019 11:24:10 -0800
Subject: [PATCH] Fix/open relative on dev (#8359)
* escape characters only on hyperlinks
* removed extra line
* added tests and changes to accomodate tests
* updates to test
* added comments
* use path.join
* format doc build error
* added comments
---
.../browser/outputs/notebookMarkdown.ts | 11 +++-
.../test/browser/notebookMarkdown.test.ts | 64 +++++++++++++++++++
2 files changed, 72 insertions(+), 3 deletions(-)
create mode 100644 src/sql/workbench/contrib/notebook/test/browser/notebookMarkdown.test.ts
diff --git a/src/sql/workbench/contrib/notebook/browser/outputs/notebookMarkdown.ts b/src/sql/workbench/contrib/notebook/browser/outputs/notebookMarkdown.ts
index 160bc68aee..caf4fbf299 100644
--- a/src/sql/workbench/contrib/notebook/browser/outputs/notebookMarkdown.ts
+++ b/src/sql/workbench/contrib/notebook/browser/outputs/notebookMarkdown.ts
@@ -59,7 +59,7 @@ export class NotebookMarkdownRenderer {
let signalInnerHTML: () => void;
const withInnerHTML = new Promise(c => signalInnerHTML = c);
- let notebookFolder = path.dirname(this._notebookURI.fsPath) + '/';
+ let notebookFolder = this._notebookURI ? path.join(path.dirname(this._notebookURI.fsPath), path.sep) : '';
if (!this._baseUrls.some(x => x === notebookFolder)) {
this._baseUrls.push(notebookFolder);
}
@@ -111,7 +111,12 @@ export class NotebookMarkdownRenderer {
text = removeMarkdownEscapes(text);
}
title = removeMarkdownEscapes(title);
- href = removeMarkdownEscapes(href);
+ // only remove markdown escapes if it's a hyperlink, filepath usually can start with .{}_
+ // and the below function escapes them if it encounters in the path.
+ // dev note: using path.isAbsolute instead of isPathLocal since the latter accepts resolver (IRenderMime.IResolver) to check isLocal
+ if (!path.isAbsolute(href)) {
+ href = removeMarkdownEscapes(href);
+ }
if (
!href
|| !markdown.isTrusted
@@ -203,7 +208,7 @@ export class NotebookMarkdownRenderer {
href = this.resolveUrl(base, href);
}
try {
- href = encodeURI(href).replace(/%5C/g, '\\').replace(/%25/g, '%');
+ href = encodeURI(href).replace(/%5C/g, '\\').replace(/%7C/g, '|').replace(/%25/g, '%');
} catch (e) {
return null;
}
diff --git a/src/sql/workbench/contrib/notebook/test/browser/notebookMarkdown.test.ts b/src/sql/workbench/contrib/notebook/test/browser/notebookMarkdown.test.ts
new file mode 100644
index 0000000000..4ab3d1ed0b
--- /dev/null
+++ b/src/sql/workbench/contrib/notebook/test/browser/notebookMarkdown.test.ts
@@ -0,0 +1,64 @@
+/*---------------------------------------------------------------------------------------------
+ * 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 * as marked from 'vs/base/common/marked/marked';
+import { NotebookMarkdownRenderer } from '../../browser/outputs/notebookMarkdown';
+import { URI } from 'vs/base/common/uri';
+
+suite('NotebookMarkdownRenderer', () => {
+ let notebookMarkdownRenderer = new NotebookMarkdownRenderer();
+ test('image rendering conforms to default', () => {
+ const markdown = { value: `` };
+ const result: HTMLElement = notebookMarkdownRenderer.renderMarkdown(markdown);
+ const renderer = new marked.Renderer();
+ const imageFromMarked = marked(markdown.value, {
+ sanitize: true,
+ renderer
+ }).trim();
+ assert.strictEqual(result.innerHTML, imageFromMarked);
+ });
+
+ test('image rendering conforms to default without title', () => {
+ const markdown = { value: `` };
+ const result: HTMLElement = notebookMarkdownRenderer.renderMarkdown(markdown);
+ const renderer = new marked.Renderer();
+ const imageFromMarked = marked(markdown.value, {
+ sanitize: true,
+ renderer
+ }).trim();
+ assert.strictEqual(result.innerHTML, imageFromMarked);
+ });
+
+ test('image width from title params', () => {
+ let result: HTMLElement = notebookMarkdownRenderer.renderMarkdown({ value: `` });
+ assert.strictEqual(result.innerHTML, `
`);
+ });
+
+ test('image height from title params', () => {
+ let result: HTMLElement = notebookMarkdownRenderer.renderMarkdown({ value: `` });
+ assert.strictEqual(result.innerHTML, `
`);
+ });
+
+ test('image width and height from title params', () => {
+ let result: HTMLElement = notebookMarkdownRenderer.renderMarkdown({ value: `` });
+ assert.strictEqual(result.innerHTML, `
`);
+ });
+
+ test('link from local file path', () => {
+ let result: HTMLElement = notebookMarkdownRenderer.renderMarkdown({ value: `[Link to File Path](someFileurl)`, isTrusted: true });
+ assert.strictEqual(result.innerHTML, `Link to File Path
`);
+ });
+
+ test('link from relative file path', () => {
+ notebookMarkdownRenderer.setNotebookURI(URI.parse('maddy/temp/file1.txt'));
+ let result: HTMLElement = notebookMarkdownRenderer.renderMarkdown({ value: `[Link to relative path](../test/.build/someimageurl)`, isTrusted: true });
+ if (process.platform === 'win32') {
+ assert.strictEqual(result.innerHTML, `Link to relative path
`);
+ } else {
+ assert.strictEqual(result.innerHTML, `Link to relative path
`);
+ }
+ });
+});