Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clicking links to local files inside a preview window does not work #435

Closed
lkydev opened this issue Aug 24, 2021 · 14 comments
Closed

Clicking links to local files inside a preview window does not work #435

lkydev opened this issue Aug 24, 2021 · 14 comments
Labels
Milestone

Comments

@lkydev
Copy link

lkydev commented Aug 24, 2021

Description

Clicking links to local files inside a preview window does not work.

System Information

Version: 1.59.1 (system setup)
Commit: 3866c3553be8b268c8a7f8c0482c0c0177aa8bfa
Date: 2021-08-19T11:56:46.957Z
Electron: 13.1.7
Chrome: 91.0.4472.124
Node.js: 14.16.0
V8: 9.1.269.36-electron.0
OS: Windows_NT x64 10.0.17763

Running asciidoctor extension 2.8.9

To Reproduce

  1. Create a folder C:\Users\<N>\Downloads\test where <N> is your user account name. (In other words, create a folder named 'test' inside your Download directory.)
  2. Open the created folder with VSCode. When asked whether you trusted the authors, click Yes, I trust the authors.
  3. Create an asciidoc file C:\Users\<N>\Downloads\test\test1.adoc. (Remember to substitute your user account name for <N>, including the content below):
= This is test1.adoc

link:test2.adoc[1]
link:.\test2.adoc[2]
link:C:\Users\<N>\Downloads\test\test2.adoc[3]
link:file:///C:\Users\<N>\Downloads\test\test2.adoc[4]
  1. Create another asciidoc file C:\Users\<N>\Downloads\test\test2.adoc:
= This is test2.adoc
  1. Open the preview window for test1.adoc. Click any links. The first two unexpectedly open a web browser window (see the screenshot). The last two do nothing.

Screenshots & Files

This is what the browser window looks like

image

Additional Context

I expect a tab to be opened for test2.adoc when any link inside test1.adoc is clicked in the preview window.

@danyill
Copy link
Contributor

danyill commented Oct 3, 2021

The fix in #455 would be functional but doesn't provide strong integration with the VS Code security model.

I think previously these links were rewritten with vscode-resource (see here) but some time in the last 6 months this became non-functional (perhaps earlier, perhaps on the insiders built? See here)

  • This upstream breakage has occurred on other extensions over the last few months (see here) for details. It seems to have occurred at about versions 1.56 - 1.57 of VS Code.
  • There was a note in the release notes for Service Workers which indicated that "edge cases" might break and that asWebViewUri should always be used and that there might be breakages in future

I'm going to comment on this issue about what I learn as I research the VS Code security model so we can provide a better integration:

Within Asciidoctor.js we can create a template that extends the default html5 converter which will process links (and other resources) where we can build the required logic (see the Docs).

I think we need to have templates which handle:

  • audio
  • image
  • video
  • inline_anchor
  • inline_image

I don't think it's possible to support arbitrary content in passthroughs.

I need to study the Markdown extension to look through implementation in more detail.

@ggrossetie
Copy link
Member

Thanks for gathering all these information.

It seems we should use asWebViewUri to encode all resources used within the HTML (or absolute paths) (see here). This is also clearly in the docs

I don't think that's true. As far as I understand, asWebViewUri should be used for linked resources such as JavaScript, CSS, fonts, images...
Regarding links, I think it's a different strategy.

In microsoft/vscode#126402 (comment), Matt Bierner suggests:

Try looking at the url in the preview (or using an absolute path: /path/to/img)

So maybe we should resolve relative paths to use absolue path, but I guess it won't work in a web environment (https://github.dev/) or use a <base> element to configure the document base URL (docdir).

@ggrossetie
Copy link
Member

ggrossetie commented Oct 3, 2021

For reference, here's what the Markdown extension generates:

# Index

![Image](./sunset.jpg)

[MDN Web Docs](https://developer.mozilla.org/)

[relative](./index.html)
<head>
<!-- ... -->
<base href="https://file%2B.vscode-resource.vscode-webview.net/c%3A/my%20documentation/index.md">
</head>
<body class=".." data-vscode-theme-name="Dark+ (default dark)">
  <h1 id="index" data-line="0" class="code-line code-active-line">Index</h1>
  <p data-line="2" class="code-line"><img src="./sunset.jpg" alt="Image" class="" id="image-hash-696037171" data-src="./sunset.jpg"></p>
  <p data-line="4" class="code-line"><a href="https://developer.mozilla.org/" data-href="https://developer.mozilla.org/" title="https://developer.mozilla.org/">MDN Web Docs</a></p>
  <p data-line="6" class="code-line"><a href="./index.html" data-href="./index.html" title="./index.html">relative</a></p>
  <!-- ... -->
</body>

So links are relative and the document has a <base> element.

Using the Asciidoctor extension:

= Index

image:sunset.jpg[]

https://developer.mozilla.org/[MDN Web Docs]

link:./index.html[relative]
<html>
<head>
  <!-- ... -->
  <base href="vscode-resource:/c:/my documentation/index.adoc">
</head>
<body class="..." data-vscode-theme-name="Dark+ (default dark)">
  <title>Index</title>
  <div id="header">
    <h1>Index</h1>
  </div>
  <div id="content">
    <div class="paragraph data-line-3">
      <p><span class="image"><img src="sunset.jpg" alt="sunset"></span></p>
    </div>
    <div class="paragraph data-line-5">
      <p><a href="https://developer.mozilla.org/" title="https://developer.mozilla.org/">MDN Web Docs</a></p>
    </div>
    <div class="paragraph data-line-7">
      <p><a href="./index.html" title="./index.html">relative</a></p>
    </div>
  </div>
  <div id="footer">
    <div id="footer-text">Last updated 2021-10-03 22:45:26 +0200</div>
  </div>
  <!-- ... -->		
</body
</html>

As you can see the href in <base> is incorrect!
So maybe, all we need to do is fix the value of the href on the <base> element.

@danyill
Copy link
Contributor

danyill commented Oct 4, 2021

For reference, here's what the Markdown extension generates:

Nice comparison.

If we're going to change the <base> element (I shall experiment) then this is likely the point at which the "Save to HTML" should generate a different output to be useful if opened in a browser on a filesystem. So far they've been able to travel together.

I guess we should keep the scope tight and discuss links here and deal with images in #436

danyill added a commit to danyill/asciidoctor-vscode that referenced this issue Oct 4, 2021
@danyill
Copy link
Contributor

danyill commented Oct 4, 2021

It seems we should use asWebViewUri to encode all resources used within the HTML (or absolute paths) (see here). This is also clearly in the docs

I don't think that's true. As far as I understand, asWebViewUri should be used for linked resources such as JavaScript, CSS, fonts, images... Regarding links, I think it's a different strategy.

In microsoft/vscode#126402 (comment), Matt Bierner suggests:

Try looking at the url in the preview (or using an absolute path: /path/to/img)

So maybe we should resolve relative paths to use absolute path, but I guess it won't work in a web environment (https://github.dev/) or use a <base> element to configure the document base URL (docdir).

You're right of course... but also the Markdown preview uses asWebviewUri to calculate the base.

And that is also insufficient. The other thing I noticed about the above and also fixes on other extensions is that they also set data-href to the value of the href attribute for links (and data-src for the image).

I have corrected the base element and prototyped using a custom converter and this also allows links to work. This works because it uses a code branch we already have in the code (copied from the Markdown extension I guess) in about L140 below:

let node: any = event.target
while (node) {
if (node.tagName && node.tagName === 'A' && node.href) {
// if (node.getAttribute('href').startsWith('#')) {
// // const fragment = node.href.split('#')[1]
// // if (fragment) {
// // location.hash = '#' + decodeURI(fragment)
// // }
// }
// Like VSCode Markdown extension, pass through some known schemes
let hrefText = node.getAttribute('data-href')
if (!hrefText) {
// Pass through known schemes
if (passThroughLinkSchemes.some((scheme) => node.href.startsWith(scheme))) {
return
}
hrefText = node.getAttribute('href')
}
// If original link doesn't look like a url, delegate back to VS Code to resolve
if (!/^[a-z-]+:/i.test(hrefText)) {
messaging.postMessage('clickLink', { href: hrefText })
event.preventDefault()
event.stopPropagation()
return
}
// Like how VSCode Markdown, pass link to backend and let it resolve it
// if (node.href.startsWith('file://') || node.href.startsWith('vscode-resource:')) {
// const [path, fragment] = node.href.replace(/^(file:\/\/|vscode-resource:)/i, '').split('#');
// messaging.postMessage('clickLink', { path, fragment });
// event.preventDefault();
// event.stopPropagation();
// break;
// }
break
}
node = node.parentNode
}
}, true)

My next task is to read code for some time to try and understand why the Markdown extension works the way it does (why put the href in two places, why is that special?). But perhaps it's good to have a similar approach to the Markdown extension in any case to more easily handle future changes.

danyill added a commit to danyill/asciidoctor-vscode that referenced this issue Oct 15, 2021
danyill added a commit that referenced this issue Oct 19, 2021
Restoring links to working in preview pane, closes #435
@tombolano
Copy link
Contributor

This issue seems to be happening again, it is happening to me using the last version of vscode (also in vscodium) and the last version of the extension (v2.9.8).

In the developers tool console I get the following error when I click on a link to a local file:

ERR cannot open file:///%5Bobject%20Object%5D.md. Detail: Unable to read file '/[object Object].md' (Error: Unable to resolve nonexistent file '/[object Object].md'): Error: cannot open file:///%5Bobject%20Object%5D.md. Detail: Unable to read file '/[object Object].md' (Error: Unable to resolve nonexistent file '/[object Object].md')
    at d.$tryOpenDocument (vscode-file://vscode-app/usr/share/code/resources/app/out/vs/workbench/workbench.desktop.main.js:2731:77104)

So for some reason the extension is trying to open the file "/[object Object].md'", I tried to create this file and indeed it is opened when I click on any link to a local file. The extension ".md" always appears here, independently of the actual path and extension of the file that I try to link.

This is the information of my system:

Version: 1.67.2
Commit: c3511e6c69bb39013c4a4b7b9566ec1ca73fc4d5
Date: 2022-05-17T18:23:40.286Z
Electron: 17.4.1
Chromium: 98.0.4758.141
Node.js: 16.13.0
V8: 9.8.177.13-electron.0
OS: Linux x64 5.17.0-1-amd64

@ggrossetie
Copy link
Member

@tombolano do you have the complete stacktrace? could you please share a minimal document and the steps to reproduce this issue?

@ggrossetie ggrossetie reopened this Jun 6, 2022
@tombolano
Copy link
Contributor

Yes, the steps to reproduce are almost the same as in the original post.

To reproduce

  • Create a folder <U>/test where <U> is your user folder (for example, C:\Users\username in Windows, /home/username in Linux)
  • Open the created folder in VSCode and trust the authors.
  • Create any text file in the folder, for example <U>/test/test1.txt
  • Create an asciidoc file <U>/test/test.adoc with the following contents (Important here!: replace <U> with your actual user folder)
= This is test.adoc

link:test.txt[1]
link:./test.txt[2]
link:<U>/test/test.txt[3]
link:file://<U>/test/test.txt[4]
  • Open the preview window for test.adoc and click any links.
    • Expected result: open the file test.txt in the editor
    • Actual result: nothing happens for any on the links.

Errors and stacktrace

When a link is clicked these are the errors that appear on the console:

console.ts:137 [Extension Host] rejected promise not handled within 1 second: Error: cannot open file:///%5Bobject%20Object%5D.md. Detail: Unable to read file '/[object Object].md' (Error: Unable to resolve nonexistent file '/[object Object].md')
E @ console.ts:137
$logExtensionHostMessage @ mainThreadConsole.ts:39
_doInvokeHandler @ rpcProtocol.ts:473
_invokeHandler @ rpcProtocol.ts:458
_receiveRequest @ rpcProtocol.ts:374
_receiveOneMessage @ rpcProtocol.ts:296
(anonymous) @ rpcProtocol.ts:161
invoke @ event.ts:575
deliver @ event.ts:779
fire @ event.ts:740
fire @ ipc.net.ts:638
_receiveMessage @ ipc.net.ts:958
(anonymous) @ ipc.net.ts:831
invoke @ event.ts:575
deliver @ event.ts:779
fire @ event.ts:740
acceptChunk @ ipc.net.ts:382
(anonymous) @ ipc.net.ts:338
O @ ipc.net.ts:60
emit @ node:events:390
addChunk @ node:internal/streams/readable:315
readableAddChunk @ node:internal/streams/readable:289
Readable.push @ node:internal/streams/readable:228
onStreamRead @ node:internal/stream_base_commons:199

console.ts:137 [Extension Host] stack trace: Error: cannot open file:///%5Bobject%20Object%5D.md. Detail: Unable to read file '/[object Object].md' (Error: Unable to resolve nonexistent file '/[object Object].md')    at d.$tryOpenDocument (vscode-file://vscode-app/usr/share/codium/resources/app/out/vs/workbench/workbench.desktop.main.js:2731:77104)
E @ console.ts:137
$logExtensionHostMessage @ mainThreadConsole.ts:39
_doInvokeHandler @ rpcProtocol.ts:473
_invokeHandler @ rpcProtocol.ts:458
_receiveRequest @ rpcProtocol.ts:374
_receiveOneMessage @ rpcProtocol.ts:296
(anonymous) @ rpcProtocol.ts:161
invoke @ event.ts:575
deliver @ event.ts:779
fire @ event.ts:740
fire @ ipc.net.ts:638
_receiveMessage @ ipc.net.ts:958
(anonymous) @ ipc.net.ts:831
invoke @ event.ts:575
deliver @ event.ts:779
fire @ event.ts:740
acceptChunk @ ipc.net.ts:382
(anonymous) @ ipc.net.ts:338
O @ ipc.net.ts:60
emit @ node:events:390
addChunk @ node:internal/streams/readable:315
readableAddChunk @ node:internal/streams/readable:289
Readable.push @ node:internal/streams/readable:228
onStreamRead @ node:internal/stream_base_commons:199

log.ts:313   ERR cannot open file:///%5Bobject%20Object%5D.md. Detail: Unable to read file '/[object Object].md' (Error: Unable to resolve nonexistent file '/[object Object].md'): Error: cannot open file:///%5Bobject%20Object%5D.md. Detail: Unable to read file '/[object Object].md' (Error: Unable to resolve nonexistent file '/[object Object].md')
    at d.$tryOpenDocument (vscode-file://vscode-app/usr/share/codium/resources/app/out/vs/workbench/workbench.desktop.main.js:2731:77104)

Additional comments

Initially I tested this on a Linux system (what I normally use), I have also tested it now on a Windows machine with the last versions of VSCode and the asciidoc-vscode extension and the results are the same.

Clearly something is wrong, but in the stacktrace it seems that only Node.js and vscode functions are called.

@ggrossetie
Copy link
Member

Thanks!
I can reproduce this issue. I think we are executing:

vscode.commands.executeCommand('_asciidoc.openDocumentLink', { path, fragment })

But the command is definitely wrong:

public execute (args: OpenDocumentLinkArgs) {
const p = decodeURIComponent(args.path)
return this.tryOpen(p, args).catch(async () => {
if (extname(p) === '') {
return this.tryOpen(p + '.md', args)
}
const resource = vscode.Uri.file(p)
await vscode.commands.executeCommand('vscode.open', resource)
return undefined
})
}

I believe that args.path is an Object and not a String, that's why the stringify value is %5Bobject%20Object%5D. As a result, extname(p) is empty because %5Bobject%20Object%5D has no extension and, in desperation, we try to open p + '.md' 😄

return this.tryOpen(p + '.md', args)

@tombolano
Copy link
Contributor

tombolano commented Jun 7, 2022

Yes, thank you, it seems that the problem is in the function onDidClickPreviewLink:

private async onDidClickPreviewLink (href: string) {
let [hrefPath, fragment] = decodeURIComponent(href).split('#')
// From Markdown plugin
if (hrefPath[0] !== '/') {
hrefPath = path.join(path.dirname(this.resource.fsPath), hrefPath)
} else {
// Handle any normalized file paths
hrefPath = vscode.Uri.parse(hrefPath.replace('/file', '')).fsPath
}
const openLinks = this.config.get<string>('preview.openAsciidocLinks', 'inPreview')
if (openLinks === 'inPreview') {
const asciidocLink = await resolveLinkToAsciidocFile(hrefPath)
if (asciidocLink) {
this.update(asciidocLink)
return
}
}
vscode.commands.executeCommand('_asciidoc.openDocumentLink', { path, fragment })
}

I have looked into this problem and the path variable in line 454 is not defined by the function (actually it is not undefined I think because of this import at the top of the file: import * as path from 'path') . This problem was introduced in commit 01620df#diff-76b777a4b16967d1f6646cacd6fb154cb50ca196a2a37cc70de0a8c3d4e9af54

So hrefPath should be used for setting the path in line 454.

Also, I noticed that there is a related subtle bug in the same function in line 445:

const openLinks = this.config.get<string>('preview.openAsciidocLinks', 'inPreview')

This line is used to decide if opening AsciiDoc files in the preview window or in the editor following the option asciidoc.preview.openAsciiDocLinks, which can be set to inPreview or to inEditor. However, the code always returns 'inPreview', thus the option has no effect and AsciiDoc files are always opened in the preview window.

The bug here is that the string in the code is wrong, the D in the option openAsciiDocLinks should be uppercase, but in that line of the code is lowercase, and thus it always returns the default value inPreview.

This is the diff with the fixes to these problems:

diff --git a/src/features/preview.ts b/src/features/preview.ts
index d235117..1a7e190 100644
--- a/src/features/preview.ts
+++ b/src/features/preview.ts
@@ -435,7 +435,7 @@ export class AsciidocPreview {
       // Handle any normalized file paths
       hrefPath = vscode.Uri.parse(hrefPath.replace('/file', '')).fsPath
     }
-    const openLinks = this.config.get<string>('preview.openAsciidocLinks', 'inPreview')
+    const openLinks = this.config.get<string>('preview.openAsciiDocLinks', 'inPreview')
     if (openLinks === 'inPreview') {
       const asciidocLink = await resolveLinkToAsciidocFile(hrefPath)
       if (asciidocLink) {
@@ -444,7 +444,7 @@ export class AsciidocPreview {
       }
     }
 
-    vscode.commands.executeCommand('_asciidoc.openDocumentLink', { path, fragment })
+    vscode.commands.executeCommand('_asciidoc.openDocumentLink', { path: hrefPath, fragment })
   }
 
   private async onCacheImageSizes (imageInfo: { id: string, width: number, height: number }[]) {

I have checked and with these fixes everything seems to be working fine.

@ggrossetie
Copy link
Member

Thanks for the thorough investigation!
Would you like to submit a pull request?

@marieflorescontact marieflorescontact added the 🔗 dependencies Pull requests that update a dependency file label Jun 9, 2022
@marieflorescontact marieflorescontact removed the 🔗 dependencies Pull requests that update a dependency file label Jun 9, 2022
@ggrossetie ggrossetie modified the milestones: 2.8.10, 3.0.0 Jun 14, 2022
@ggrossetie
Copy link
Member

Fixed in #572
@marieflorescontact also fixed a few issues regarding paths starting with file:/// or containing file/ in #573

@jowitte
Copy link

jowitte commented Aug 18, 2022

Happens to me in v2.9.8. Am I doing something wrong?

@tombolano
Copy link
Contributor

Happens to me in v2.9.8. Am I doing something wrong?

No, nothing wrong, the bug was actually fixed in the 3.0.0 version, last version is 3.0.1 (see releases: https://github.com/asciidoctor/asciidoctor-vscode/releases), however these versions are marked as prereleases, that's why in the front page still 2.9.8 appears as the last version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
6 participants