Fix images in Notebook not showing (#18160)

* Fix images in Notebook not showing

* fixes

* more fixes
This commit is contained in:
Charles Gagnon
2022-01-27 15:00:00 -08:00
committed by GitHub
parent 8333a39615
commit ad06fc7321
5 changed files with 65 additions and 12 deletions

View File

@@ -27,6 +27,7 @@ import { TextCellEditModes } from 'sql/workbench/services/notebook/common/contra
import { NotebookLinkHandler } from 'sql/workbench/contrib/notebook/browser/notebookLinkHandler';
import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent';
import { KeyCode } from 'vs/base/common/keyCodes';
import { FileAccess } from 'vs/base/common/network';
export const MARKDOWN_TOOLBAR_SELECTOR: string = 'markdown-toolbar-component';
const linksRegex = /\[(?<text>.+)\]\((?<url>[^ ]+)(?: "(?<title>.+)")?\)/;
@@ -297,8 +298,12 @@ export class MarkdownToolbarComponent extends AngularDisposable {
await insertFormattedMarkdown(linkCalloutResult?.insertEscapedMarkdown, this.getCellEditorControl());
} else if (type === MarkdownButtonType.IMAGE_PREVIEW) {
if (imageCalloutResult.embedImage) {
let base64String = await this.getFileContentBase64(URI.file(imageCalloutResult.imagePath));
let mimeType = await this.getFileMimeType(URI.file(imageCalloutResult.imagePath));
// VS Code blocks loading directly from the file protocol - we have to transform it to a vscode-file URI
// first. Currently we assume that the path here is always going to be a path since we don't support
// embedding images from web links.
const uri = FileAccess.asBrowserUri(URI.file(imageCalloutResult.imagePath));
let base64String = await this.getFileContentBase64(uri);
let mimeType = await this.getFileMimeType(uri);
const originalImageName: string = path.basename(imageCalloutResult.imagePath).replace(/\s/g, '');
let attachmentName = this.cellModel.addAttachment(mimeType, base64String, originalImageName);
if (originalImageName !== attachmentName) {

View File

@@ -9,6 +9,7 @@ import * as path from 'vs/base/common/path';
import * as turndownPluginGfm from 'sql/workbench/contrib/notebook/browser/turndownPluginGfm';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { findPathRelativeToContent, NotebookLinkHandler } from 'sql/workbench/contrib/notebook/browser/notebookLinkHandler';
import { FileAccess } from 'vs/base/common/network';
// These replacements apply only to text. Here's how it's handled from Turndown:
// if (node.nodeType === 3) {
@@ -121,9 +122,11 @@ export class HTMLMarkdownConverter {
filter: 'img',
replacement: (content, node) => {
if (node?.src) {
let imgPath = URI.parse(node.src);
// Image URIs are converted to vscode-file URIs for the underlying HTML so that they can be loaded by ADS,
// but we want to convert them back to their file URI when converting back to markdown for displaying to the user
let imgUri = FileAccess.asFileUri(URI.parse(node.src));
const notebookFolder: string = this.notebookUri ? path.join(path.dirname(this.notebookUri.fsPath), path.sep) : '';
let relativePath = findPathRelativeToContent(notebookFolder, imgPath);
let relativePath = findPathRelativeToContent(notebookFolder, imgUri);
if (relativePath) {
return `![${node.alt}](${relativePath})`;
}

View File

@@ -17,6 +17,7 @@ import { IMarkdownStringWithCellAttachments, MarkdownRenderOptionsWithCellAttach
import { replaceInvalidLinkPath } from 'sql/workbench/contrib/notebook/common/utils';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { useNewMarkdownRendererKey } from 'sql/workbench/contrib/notebook/common/notebookCommon';
import { FileAccess, Schemas } from 'vs/base/common/network';
// Based off of HtmlContentRenderer
export class NotebookMarkdownRenderer {
@@ -98,7 +99,18 @@ export class NotebookMarkdownRenderer {
}
let attributes: string[] = [];
if (href) {
attributes.push(`src="${href}"`);
// VS Code blocks loading directly from the file protocol - we have to transform it to a vscode-file URI
// first. Since the href here can be either a file path, an HTTP/S link or embedded data though we first
// check if it's any of the others and if so then don't need to do anything.
let uri = URI.parse(href);
if (!(uri.scheme === Schemas.https ||
uri.scheme === Schemas.http ||
uri.scheme === Schemas.data ||
uri.scheme === Schemas.attachment ||
uri.scheme === Schemas.vscodeFileResource)) {
uri = FileAccess.asBrowserUri(URI.file(href));
}
attributes.push(`src="${uri.toString(true)}"`);
}
if (text) {
attributes.push(`alt="${text}"`);

View File

@@ -18,7 +18,7 @@ suite('NotebookMarkdownRenderer', () => {
const imageFromMarked = marked(markdown.value, {
sanitize: true,
renderer
}).trim();
}).trim().replace('someimageurl', 'vscode-file://vscode-app/someimageurl');
assert.strictEqual(result.innerHTML, imageFromMarked);
});
@@ -26,26 +26,26 @@ suite('NotebookMarkdownRenderer', () => {
const markdown = { value: `![image](someimageurl)` };
const result: HTMLElement = notebookMarkdownRenderer.renderMarkdown(markdown);
const renderer = new marked.Renderer();
const imageFromMarked = marked(markdown.value, {
let imageFromMarked = marked(markdown.value, {
sanitize: true,
renderer
}).trim();
}).trim().replace('someimageurl', 'vscode-file://vscode-app/someimageurl');
assert.strictEqual(result.innerHTML, imageFromMarked);
});
test('image width from title params', () => {
let result: HTMLElement = notebookMarkdownRenderer.renderMarkdown({ value: `![image](someimageurl|width=100 'caption')` });
assert.strictEqual(result.innerHTML, `<p><img src="someimageurl" alt="image" title="caption" width="100"></p>`);
assert.strictEqual(result.innerHTML, `<p><img src="vscode-file://vscode-app/someimageurl" alt="image" title="caption" width="100"></p>`);
});
test('image height from title params', () => {
let result: HTMLElement = notebookMarkdownRenderer.renderMarkdown({ value: `![image](someimageurl|height=100 'caption')` });
assert.strictEqual(result.innerHTML, `<p><img src="someimageurl" alt="image" title="caption" height="100"></p>`);
assert.strictEqual(result.innerHTML, `<p><img src="vscode-file://vscode-app/someimageurl" alt="image" title="caption" height="100"></p>`);
});
test('image width and height from title params', () => {
let result: HTMLElement = notebookMarkdownRenderer.renderMarkdown({ value: `![image](someimageurl|height=200,width=100 'caption')` });
assert.strictEqual(result.innerHTML, `<p><img src="someimageurl" alt="image" title="caption" width="100" height="200"></p>`);
assert.strictEqual(result.innerHTML, `<p><img src="vscode-file://vscode-app/someimageurl" alt="image" title="caption" width="100" height="200"></p>`);
});
test('link from local file path', () => {
@@ -99,12 +99,43 @@ suite('NotebookMarkdownRenderer', () => {
assert.strictEqual(result.innerHTML, `<p><img src="attachment:ads.png" alt="altText"></p>`, 'Cell attachment name not found failed');
result = notebookMarkdownRenderer.renderMarkdown({ value: `![altText](attachments:ads.png)`, isTrusted: true }, { cellAttachments: JSON.parse('{"ads2.png":{"image/png":"iVBORw0KGgoAAAANSUhEUgAAAggg=="}}') });
assert.strictEqual(result.innerHTML, `<p><img src="attachments:ads.png" alt="altText"></p>`, 'Cell attachment scheme mismatch failed');
assert.strictEqual(result.innerHTML, `<p><img src="vscode-file://vscode-app/attachments:ads.png" alt="altText"></p>`, 'Cell attachment scheme mismatch failed');
result = notebookMarkdownRenderer.renderMarkdown({ value: `![altText](attachment:ads.png)`, isTrusted: true }, { cellAttachments: JSON.parse('{"ads2.png":"image/png"}') });
assert.strictEqual(result.innerHTML, `<p><img src="attachment:ads.png" alt="altText"></p>`, 'Cell attachment no image data failed');
});
suite('Schema validation', function () {
test('https', () => {
const result: HTMLElement = notebookMarkdownRenderer.renderMarkdown({ value: `![](https://example.com)` });
assert.strictEqual(result.innerHTML, `<p><img src="https://example.com/"></p>`, 'HTTPS schema link should not be modified');
});
test('http', () => {
const result: HTMLElement = notebookMarkdownRenderer.renderMarkdown({ value: `![](http://example.com)` });
assert.strictEqual(result.innerHTML, `<p><img src="http://example.com/"></p>`, 'HTTP schema link should not be modified');
});
test('attachment & data', () => {
const result = notebookMarkdownRenderer.renderMarkdown({ value: `![altText](attachment:ads.png)`, isTrusted: false }, { cellAttachments: JSON.parse('{"ads.png":{"image/png":"iVBORw0KGgoAAAANSUhEUgAAAggg=="}}') });
assert.strictEqual(result.innerHTML, `<p><img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAggg==" alt="altText"></p>`, 'Cell with attachment should have attachment URI not be modified');
});
test('vscode-file', () => {
const result: HTMLElement = notebookMarkdownRenderer.renderMarkdown({ value: `![](vscode-file://vscode-app/mypath)` });
assert.strictEqual(result.innerHTML, `<p><img src="vscode-file://vscode-app/mypath"></p>`, 'vscode-file schema link should not be modified');
});
test('file', () => {
const filePath = URI.parse(__filename).fsPath;
const result: HTMLElement = notebookMarkdownRenderer.renderMarkdown({ value: `![](${filePath})` });
let renderedPath = filePath.replace(/\\/g, '/');
// Unix filesystems start with / which is deduplicated in the final src URI - so just remove that now if it exists
renderedPath = renderedPath.startsWith('/') ? renderedPath.slice(1) : renderedPath;
assert.strictEqual(result.innerHTML, `<p><img src="vscode-file://vscode-app/${renderedPath}"></p>`, 'file links should have vscode-file schema added');
});
test('unknown', () => {
const result: HTMLElement = notebookMarkdownRenderer.renderMarkdown({ value: `![](unknown://some/path)` });
assert.strictEqual(result.innerHTML, `<p><img src="vscode-file://vscode-app/unknown://some/path"></p>`, 'unknown schema should be treated like a path and have vscode-file schema added');
});
});
test('table followed by blank line with space and then header renders correctly (#16245)', function (): void {
let result: HTMLElement = notebookMarkdownRenderer.renderMarkdown({ value: '<table></table>\n \n### Hello', isTrusted: true });
assert.strictEqual(result.innerHTML, '<table></table>\n \n<h3 id="hello">Hello</h3>\n');