diff --git a/src/sql/workbench/contrib/notebook/browser/notebookLinkHandler.ts b/src/sql/workbench/contrib/notebook/browser/notebookLinkHandler.ts index cae0675166..d645fc63ad 100644 --- a/src/sql/workbench/contrib/notebook/browser/notebookLinkHandler.ts +++ b/src/sql/workbench/contrib/notebook/browser/notebookLinkHandler.ts @@ -12,7 +12,6 @@ import { isWindows } from 'vs/base/common/platform'; import { containsEncodedUriComponentReservedCharacters } from 'sql/base/common/network'; const useAbsolutePathConfigName = 'notebook.useAbsoluteFilePaths'; - export class NotebookLinkHandler { private _notebookUriLink: URI; private _href: string; @@ -53,11 +52,13 @@ export class NotebookLinkHandler { this.isMarkdown = this._link.attributes['is-markdown']?.nodeValue === 'true' ? true : false; this.isEncoded = this._link.attributes['is-encoded']?.nodeValue === 'true' ? true : false; this._isFile = this._link.protocol === `${Schemas.file}:` || this._link.protocol === `${Schemas.vscodeFileResource}:`; - // Given an anchor element for windows href link will need to use nodeValue instead as that does not encode the url + // When editing on richtext, for local files paths the nodeValue results are ambiguous depending on OS + // ex on MacOS: '/PathToParentFolder/ ./fiename.ipynb', path.normalize resolves this. if (isWindows) { - this._href = this.isMarkdown || this.isEncoded ? this._link.href?.replace(/%5C/g, '\\') : this._link.attributes['href']?.nodeValue; + // Given an anchor element for windows href link will need to use nodeValue instead as that does not encode the url + this._href = this.isMarkdown || this.isEncoded ? path.normalize(this._link.href?.replace(/%5C/g, '\\')) : this._link.attributes['href']?.nodeValue; } else { - this._href = this._link.attributes['href']?.nodeValue; + this._href = this._isFile ? path.normalize(this._link.attributes['href']?.nodeValue) : this._link.attributes['href']?.nodeValue; } this._notebookUriLink = this._href ? URI.parse(encodeURI(this._href)) : undefined; this._isAnchorLink = this._notebookUriLink?.fragment ? true : false; @@ -173,9 +174,17 @@ export class NotebookLinkHandler { */ export function findPathRelativeToContent(notebookFolder: string, contentPath: URI | undefined, isMarkdown?: boolean, isEncoded?: boolean): string { if (contentPath?.scheme === 'file') { - let relativePath = contentPath.fragment ? path.relative(notebookFolder, contentPath.fsPath).concat('#', contentPath.fragment) : path.relative(notebookFolder, contentPath.fsPath); + let decodePath: boolean = contentPath.fsPath !== decodeURI(contentPath.fsPath); + let decodedContentPath = contentPath.fsPath; + if (decodePath) { + decodedContentPath = decodeURI(contentPath.fsPath); + } + // use the decodedContentPath for calculating the relative path since the notebookFolder is not encoded to get the relative path correctly. + let relativePath = contentPath.fragment ? path.relative(notebookFolder, decodedContentPath).concat('#', contentPath.fragment) : path.relative(notebookFolder, decodedContentPath); // if relativePath contains improper directory format due to marked js parsing returning an invalid path (ex. ....\) then we need to replace it to ensure the directories are formatted properly (ex. ..\..\) relativePath = isMarkdown || isEncoded ? replaceInvalidLinkPath(relativePath) : encodeURI(replaceInvalidLinkPath(relativePath)).replace(/%5C/g, '\\'); + // encode the path if we used decoded contentPath for calculating the relativePath. + relativePath = decodePath ? encodeURI(relativePath) : relativePath; if (relativePath.startsWith(path.join('..', path.sep)) || relativePath.startsWith(path.join('.', path.sep))) { return relativePath; } else { diff --git a/src/sql/workbench/contrib/notebook/test/browser/notebookLinkHandler.test.ts b/src/sql/workbench/contrib/notebook/test/browser/notebookLinkHandler.test.ts index 5f042f8e54..287a43a40d 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/notebookLinkHandler.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/notebookLinkHandler.test.ts @@ -130,4 +130,37 @@ suite('Noteboook Link Handler', function (): void { assert.strictEqual(notebookLinkHandler.getEncodedLinkUrl(), `/Notebooks/Test_Paths/My%2520File.ipynb`, '% in file path failed to be encoded'); }); }); + test('getLinkUrl should return relativePath correctly', () => { + test('when given an relative link with file protocol and useAbsoluteFilePaths set to true', () => { + let node = Object.assign(document.createElement('a'), { href: '/tmp//notebook1.ipynb', attributes: { href: { nodeValue: '/tmp/.\\notebook1.ipynb' } } }); + configurationService.updateValue('notebook.useAbsoluteFilePaths', true, ConfigurationTarget.USER); + node.setAttribute("protocol", 'file:'); + let notebookLinkHandler = new NotebookLinkHandler(notebookUri, node, configurationService); + let expectedResult = `.${path.join(path.sep, 'notebook1.ipynb')}` + assert.strictEqual(notebookLinkHandler.getLinkUrl(), expectedResult, 'File relative link is wrong'); + }); + test('when given an relative link with vscode-file protocol', () => { + let node = Object.assign(document.createElement('a'), { href: '/tmp//notebook1.ipynb', attributes: { href: { nodeValue: '/tmp/.\\notebook1.ipynb' } } }); + node.setAttribute("protocol", 'vscode-file:'); + let notebookLinkHandler = new NotebookLinkHandler(notebookUri, node, configurationService); + let expectedResult = `.${path.join(path.sep, 'notebook1.ipynb')}` + assert.strictEqual(notebookLinkHandler.getLinkUrl(), expectedResult, 'File relative link is wrong'); + }); + test('when is-encoded is true', () => { + let result = new NotebookLinkHandler(notebookUri, Object.assign(document.createElement('a'), { href: '/tmp/stuff.png', attributes: { isEncoded: true, isMarkdown: true } }), configurationService); + assert.strictEqual(result.getLinkUrl(), `.${path.sep}stuff.png`, 'Basic link test failed'); + + result = new NotebookLinkHandler(notebookUri, Object.assign(document.createElement('a'), { href: '/stuff.png', attributes: { isEncoded: true, isMarkdown: true } }), configurationService); + assert.strictEqual(result.getLinkUrl(), `..${path.sep}stuff.png`, 'Basic link test above folder failed'); + + result = new NotebookLinkHandler(notebookUri, Object.assign(document.createElement('a'), { href: '/tmp/inner/stuff.png', attributes: { isEncoded: true, isMarkdown: true } }), configurationService); + assert.strictEqual(result.getLinkUrl(), `.${path.sep}inner${path.sep}stuff.png`, 'Basic link test below folder failed'); + + result = new NotebookLinkHandler(notebookUri, Object.assign(document.createElement('a'), { href: '/tmp/my stuff.png', attributes: { isEncoded: true, isMarkdown: true } }), configurationService); + assert.strictEqual(result.getLinkUrl(), `.${path.sep}my%20stuff.png`, 'Basic link test with space filename failed'); + + result = new NotebookLinkHandler(notebookUri, Object.assign(document.createElement('a'), { href: '/tmp/my%20stuff.png', attributes: { isEncoded: true, isMarkdown: true } }), configurationService); + assert.strictEqual(result.getLinkUrl(), `.${path.sep}my%2520stuff.png`, 'Basic link test with %20 filename failed'); + }) + }); });