Desktop: Handle URL links

[roman-r-m]

Copy of #5302 that was merged by accident and subsequently reverted.

[laurent22]

It doesn't work on macOS. Maybe you'll have more luck with the fileAssociation property: https://www.electron.build/configuration/configuration#overridable-per-platform-options

[roman-r-m]

Can we just say that this change is only for Win & Linux and if someone else wants and can implement this for MacOS, they can do it separately?
Because I don't think it's feasible - me making a change without a way to test it, pushing it, and waiting for someone else to test.

[laurent22]

Hmm, I'm not sure because it's one of those things that are all or nothing. If we don't support it for macOS, we'll have bug reports all the time explaining that it doesn't work, or people building workflow around this just to find out that it's not actually cross-platform.

However, we can leave the pull request open and see if someone can look into this at some point.

[tessus]

Sometimes macOS has to be rebooted, because plist files are cached and it doesn't seem to be possible to wipe the cache.

I'll run a few tests tomorrow. Let's see what I can find out.

[tessus]

I ran some tests. I've added one additional line to the plist file: https://github.com/roman-r-m/joplin/pull/2/files

Opening a valid joplin:// link starts Joplin but does not do anything after Joplin has started. When Joplin is already running, everything works as expected. It opens a note, a notebook, ...

So for some reason the callback URI code is not invoked when Joplin is started. Maybe I can find out more tomorrow.

[laurent22]

Looks like it's disabled for macOS when Joplin is already running:

https://github.com/laurent22/joplin/pull/5416/files#diff-24d0a98085bfa7f033de08145708a9c184c2eed3cadfb6d19f019ba2b79c741cR336

Can we remove this conditional now that macOS is supported?

[tessus]

Yep, I saw that too. But it's working when Joplin is running. It's not when it is not running (except that Joplin is started). So this code is never touched anyway. Very confusing. But maybe I'm missing something. I'm gonna look at it again tomorrow.

[roman-r-m]

Can we remove this conditional now that macOS is supported?

I'll need to double-check but I recall the reason for this condition is not to disable because it's not supported on MacOS but rather that on MacOS instead of second-instance open-url is called.

[tessus]

It must be that open-url is called, otherwise a joplin link wouldn't work when Joplin is running. I'm still trying to figure out why it seems to be ignored at startup.

[roman-r-m]

Found this in Electron's docs (https://github.com/electron/electron/blob/v10.4.7/docs/api/app.md#apprequestsingleinstancelock)

On macOS, the system enforces single instance automatically when users try to open a second instance of your app in Finder, and the open-file and open-url events will be emitted for that. However when users start your app in command line, the system's single instance mechanism will be bypassed, and you have to use this method to ensure single instance.

[tessus]

I've been trying a few things since this morning. First I fixed a mssing void for the Promise in ElectronWrapper.ts.
I also used the PreventDefault on the event.

I'll look into the link you sent next. Btw, your test !== darwin should have also included Linux, since both of them are the same when it comes to open-url behavior so I would have looked into that next.

[roman-r-m]

I'll look into the link you sent next. Btw, your test !== darwin should have also included Linux, since both of them are the same when it comes to open-url behavior so I would have looked into that next.

It works for me on Linux but maybe it goes via the other callback.

[tessus]

I've tried a few things (see my commits https://github.com/roman-r-m/joplin/pull/2/commits) but nothing worked. Maybe there's a bug in Electron.

The following

On macOS, the system enforces single instance automatically when users try to open a second instance of your app in Finder, and the open-file and open-url events will be emitted for that. However when users start your app in command line, the system's single instance mechanism will be bypassed, and you have to use this method to ensure single instance.

does not mean that open-url is not called, does it? It just means that you have to get the single instance lock manually on macOS for command line invocation.

Right now I'm not sure what else to try. Maybe I'm missing something obvious. I've been a bit off the last couple of days. I've caught a cold and my concentration is lacking.

[roman-r-m]

For macOS I was mainly using this blog post https://shipshape.io/blog/launch-electron-app-from-browser-custom-protocol/
I've just looked again and the author suggests adding this for electron-builder to create valid plist

"build": {
  "protocols": [
    {
      "name": "custom",
      "schemes": [
        "custom"
      ]
    }
  ]
}

[tessus]

The plist file looks perfectly valid. I believe protocols section might just be another form of the extendInfo part. But I can try that too.

[roman-r-m]

Just to confirm - were running joplin via npm start? If so could you try building a proper release distribution and testing this way?
Neither on Windows nor on Linux it does not work when running in dev.

[tessus]

No, I'm building a proper app and testing it in a VM.

[tessus]

That didn't work either. Nor did setting setAsDefaultProtocolClient help. I'm running out of options.

[tessus]

I really have no idea why it is not working. I think I've tried all possibilities as you can see from my commits. After every commit I created a new package and ran several tests.

Maybe there's a problem with the wrapper, or this has to be put in main.js. Or there is a problem with this Electron version. I think we need help from an Electron expert.

[roman-r-m]

Maybe there's a problem with the wrapper, or this has to be put in main.js. Or there is a problem with this Electron version. I think we need help from an Electron expert.

Right. I've no idea either. On all other systems it works fine so must be something specific to macOS. I'll ask on the forum, maybe someone is familiar with this.

[laurent22]

Maybe the fileAssociations parameter would work since they might have already fixed various edge cases?

https://www.electron.build/configuration/configuration#overridable-per-platform-options

[tessus]

I can try that tomorrow as well. But I think these options only tell the builder to create a valid plist file, which is currently already perfectly fine. But we'll see. Fingers crossed.

[tessus]

This did not work at all:

  ⨯ Invalid configuration object. electron-builder 22.9.1 has been initialized using a configuration object that does not match the API schema.
 - configuration.fileAssociations should be one of these:
   object { ext, description?, icon?, isPackage?, mimeType?, name?, rank?, role? } | [object { ext, description?, icon?, isPackage?, mimeType?, name?, rank?, role? }, ...]
   -> The file associations.
   Details:
    * configuration.fileAssociations[0] has an unknown property 'protocols'. These properties are valid:
      object { ext, description?, icon?, isPackage?, mimeType?, name?, rank?, role? }
      -> File associations.

      macOS (corresponds to [CFBundleDocumentTypes](https://developer.apple.com/library/content/documentation/General/Reference/InfoPlistKeyReference/Articles/CoreFoundationKeys.html#//apple_ref/doc/uid/20001431-101685)) and NSIS only.

      On Windows works only if [nsis.perMachine](https://electron.build/configuration/configuration#NsisOptions-perMachine) is set to `true`.
    * configuration.fileAssociations[0] has an unknown property 'schemes'. These properties are valid:
      object { ext, description?, icon?, isPackage?, mimeType?, name?, rank?, role? }
      -> File associations.

      macOS (corresponds to [CFBundleDocumentTypes](https://developer.apple.com/library/content/documentation/General/Reference/InfoPlistKeyReference/Articles/CoreFoundationKeys.html#//apple_ref/doc/uid/20001431-101685)) and NSIS only.

      On Windows works only if [nsis.perMachine](https://electron.build/configuration/configuration#NsisOptions-perMachine) is set to `true`.
    * configuration.fileAssociations[0] misses the property 'ext'. Should be:
      [string, ...] | string
      -> The extension (minus the leading period). e.g. `png`.
     How to fix:
     1. Open https://www.electron.build/configuration/configuration
     2. Search the option name on the page (or type in into Search to find across the docs).
       * Not found? The option was deprecated or not exists (check spelling).
       * Found? Check that the option in the appropriate place. e.g. "title" only in the "dmg", not in the root.

Apparently the extension property is required, but I don't need a file extension for Joplin.

For some reason it seems that open-url is only called when the app is already running. It is not triggered when starting the app. I've tried several different things (as can be seen by the commits), but nothing worked.

[roman-r-m]

For some reason it seems that open-url is only called when the app is already running. It is not triggered when starting the app. I've tried several different things (as can be seen by the commits), but nothing worked.

Why don't we agree that this is a good first step and release what works.

EDIT
Just remembered I was going to ask on the forum too.

[tessus]

Btw, you should cherry-pick the first 2 commits from my PR based on yours....
They are not solving the problem, but make the code better.

[roman-r-m]

Btw, you should cherry-pick the first 2 commits from my PR based on yours....
They are not solving the problem, but make the code better.

Thanks.

[JGCarroll]

roman-r-m#3

Thought I'd tag on an update to the Linux updater script, the use of echo -e ...\n\..\..\n seemed a bit excessive to me, so I replaced it with a Bash heredoc. Since there's variables used in the Heredoc, unfortunately Bash requires using tabs instead of spaces which is different to the rest of the script, but I'd argue that it's preferrable to the current line.

Edit: Looking at it a second time, is the >> (output redirect, append mode) in the original script actually intended? Surely it should be > (output redirect, use empty file)

[roman-r-m]

Apologies for the delay on this, I was on holidays.
I've just merged the latest dev and re-tested, all works fine.

Not sure what to do about macOS, can we release this just for Linux and Windows?

[laurent22]

Ok I've finally had time to try on macOS and so far that's what I found:

• It works when the app is already open - in this case, it brings the app to the front and select the right note

• It half works when the app is not opened - it opens the app but doesn't select the note. In my test, the URL was not present in process.argv so it makes sense it's not found. I'm not sure why macOS doesn't pass it - it might be a bug in the operating system, or Electron or Electron builder.

Anyway I think that's good enough for now.

I've added a few more comments and also please rebase the branch as it's currently broken (because dev was broken) due to package paths not being right.

Also please could you add the doc under /readme/external_links? If you could briefly describe what's supported and also the known bugs, like the partial support for macOS that would be great. Also please link to it from the TOC in /README.md

[laurent22]

Should "event" because the underscore is only when the variable is unused (to disable TypeScript type checking)

[roman-r-m]

done

[laurent22]

"event"

[roman-r-m]

It is unused here, and so if I remove the underscore I get a warning from the linter

[laurent22]

"==="

[roman-r-m]

also fixed

[laurent22]

Let's use "org.joplinapp.x-callback-url" as identifier.

[roman-r-m]

fixed

[laurent22]

Please add test units for this file. See other .test.ts files to see how it's done.

[roman-r-m]

this is done

[tessus]

@laurent22

It works when the app is already open - in this case, it brings the app to the front and select the right note

This worked since the beginning. I'm sorry if I didn't mention this clearly enough when I tested and tried to fix it.

the URL was not present in process.argv so it makes sense it's not found.

I still hope that someone will help out with this. Unfortunately there haven't been any replies on the forum yet.
But I also think that we should merge this.

[roman-r-m]

Also please could you add the doc under /readme/external_links? If you could briefly describe what's supported and also the known bugs, like the partial support for macOS that would be great. Also please link to it from the TOC in /README.md

This is done.

Note that the link isn't working at the moment because it refers to dev branch: https://github.com/laurent22/joplin/blob/dev/readme/external_links.md).
Once this PR is merged, it should be ok.

[laurent22]

Ok we can merge this too then. Thanks a lot @roman-r-m and @tessus! Lots of great pull requests lately.

[laurent22]

@roman-r-m, I'm writing the release notes for version 2.5 and I was wondering what would be the use case for the callback URLs? So far I mentioned that it's great to allow third party apps to open notes or notebooks, but is there a use for end users? In fact, I'm even wondering what could be the use for the "Copy external link" menu item that we've added?

[roman-r-m]

For instance I am using a different app for managing tasks and when creating a new task I can put links to relevant notes into the task description. Then when I get to that task I can immediately open the note (and start Joplin if it's not running) from there.

Even better, if we could create links to search, I could put such link into my weekly review task to instantly show all notes created in the last 7 days.

[roman-r-m]

There may be more ideas here https://discourse.joplinapp.org/t/are-url-links-on-the-roadmap/599

[laurent22]

That makes sense, thanks for clarifying!

[Beurling]

can it work in ios now?

[roman-r-m]

can it work in ios now?

This PR is only for desktop (Windows, Linux and Mac OS) so no. However, I think there was a similar PR for mobile but I can't find it right now

[Beurling]

can it work in ios now?

This PR is only for desktop (Windows, Linux and Mac OS) so no. However, I think there was a similar PR for mobile but I can't find it right now

Thanks, hope it will come sonn