Summary
I created this topic to share my proposed code changes to fix the Internal HTML notelinks issue.
What Causes the Issue
We have this issue because the link_open
rule that markdownIt uses to add js to internal note links to make them work is not applied to links written in html notation. This is because they have a token type of html_inline
, not link_open
.
I added a console.log()
to the link_open
code and found out that it only runs for regular md links and not html notation links.
So a regular md note link would be rendered as:
<p><a data-from-md data-resource-id='60ee5b0b0835484a887bb76e5bf9d109' href='#' ontouchstart='if (typeof(t) !== "undefined" && !!t) { clearTimeout(t); t = null; } else { t = setTimeout(() => { t = null; window.joplinPostMessage_("longclick:60ee5b0b0835484a887bb76e5bf9d109"); }, 500); }' ontouchend='if (!!t) {clearTimeout(t); t=null; window.joplinPostMessage_("joplin://60ee5b0b0835484a887bb76e5bf9d109");}' ontouchcancel='if (!!t) {clearTimeout(t); t=null; ontouchmove="if (!!t) {clearTimeout(t); t=null;'><span class="resource-icon fa-joplin"></span>First Note</a></p>
But a note link using html notation would be rendered as:
<p><a href=":/60ee5b0b0835484a887bb76e5bf9d109" class="jop-noMdConv">First Note</a></p>
I haven't looked into the desktop code, so I don't know why the desktop app can open internal note links regardless of this. I'd be happy to get an explanation for that.
Proposed Fix
I propose fixing this issue by adding a new html_inline
rule. The rule would check if the content of the token is an opening anchor tag using a regular expression and then use the link_replacement
function to add js to the link similar to how the link_open
rule does it.
However, I'll also need to add an isHtmlLink
option to the link_replacement
function to prevent it from adding the data-from-md
attribute to a link written in html notation and instead add the jop-noMdConv
class to it (since the current link will be replaced).
I hope this makes sense. I'd love to get feedback on this, so I can move forward with fixing the issue.
I think it would be interesting to see why it works in the desktop client, maybe code share is possible, then it would be less maintenance.
2 Likes
I agree with that, hopefully we can use the same method as on the desktop app.
1 Like
I would take a look at it, but I don't know electron at the moment.
In this particular case it won't be Electron-specific I think, just React and regular TS/JS. Likewise you can use console statements along with the dev console to find out what's happening.
I've had some issues building the desktop app on my windows machine.
# This file contains the result of Yarn building a package (@joplin/app-desktop@workspace:packages/app-desktop)
# Script name: postinstall
[e[90m16:54:16e[39m] Using gulpfile ~\Desktop\Github\joplin\packages\app-desktop\gulpfile.js
[e[90m16:54:16e[39m] Starting 'build'...
[e[90m16:54:16e[39m] Starting 'compileScripts'...
[e[90m16:54:16e[39m] Starting 'compilePackageInfo'...
[e[90m16:54:16e[39m] Starting 'copyPluginAssets'...
[e[90m16:54:16e[39m] Starting 'copyApplicationAssets'...
[e[90m16:54:16e[39m] Starting 'updateIgnoredTypeScriptBuild'...
[e[90m16:54:16e[39m] Starting 'buildCommandIndex'...
[e[90m16:54:16e[39m] Starting 'compileSass'...
Compiling C:\Users\Tolulope Malomo\Desktop\Github\joplin\packages\app-desktop\tools/../gui/ExtensionBadge.jsx...
Malomo\Desktop\Github\joplin\packages\app-desktop\tools/../gui/ExtensionBadge.min.js doesn't exist. Malomo\Desktop\Github\joplin\packages\app-desktop\tools/../gui/ExtensionBadge.jsx doesn't exist
[e[90m16:54:26e[39m] The following tasks did not complete: build, compileScripts, compilePackageInfo, copyPluginAssets, copyApplicationAssets, updateIgnoredTypeScriptBuild, buildCommandIndex, compileSass
[e[90m16:54:26e[39m] Did you forget to signal async completion?
The ExtensionBadge.jsx
does exist.
I'm not sure, but it's most likely because of the whitespace in my PC's username. I've had alot of other issues in the past because of the whitespace in my username. The only way to fix this is by doing a fresh install of windows and changing my username.
Use a path without a space, the space leads to problems.
The space is in my PC username, which I can only change by doing a fresh install of windows.
You can use a other path than your user profile. Like c:\git\joplin ...
Oh. I'll try that. Thank you.
That seemed to do the trick. Thank you!
Update
It'll take a while for me to figure out how the desktop app handles the note links, but I came up with a simple fix.
The react-native-webview
has a onNavigationStateChange
prop that can be used to intercept a user tapping on a link in the webview and do something different than navigating there in the webview.
We can use it to check if the url of the link that was clicked is a note resource and pass the url to onJoplinLinkClick() if it is.
The goal is not just to fix the bug, but to fix it well. We know there's always an easy way out or hack that can be added to get things working, but we want to avoid that because obviously that doesn't scale.
So at a minimum we need to know why it works in desktop and not mobile, when the two share a good part of the code. Then we can know which solution is the most appropriate. Hopefully it's just a simple fix once you identify where the discrepancy comes from.
1 Like