Note links

[lenzls]

Hey everybody,

so I have now a working version of the note-links proposal from #623 . But due to a bug in rxMarkdown things got a little hacky…

Details

Links that do not start with /https?/ are ignored by rxMarkdown. It completely ignores constructs like [link](example.com) and therefore also [link](356675) (1). That also means that I can't hook into the onLinkClick callback to open a note-link in the app.

In order to work around this, I introduced the following workaround:

Before the markdown gets passed to rxMarkdown I traverse through and replace all links of the format [<some-text>](<some-existing-note-file-id) with
[<some-text>](https://nextcloudnotes/notes/<some-existing-note-file-id>). Basically I add a dummy prefix to the ids to create valid urls.

Then the markdown string gets passed to rxMarkdown as usual. Since these note-links now start with https, rxMarkdown correctly interprets them as links.

Now I can hook into the onLinkClick callback and on every link click I execute the logic:

• Is the link of the structure https://nextcloudnotes/notes/<someId> then open it in the nextcloud-notes application.
• Is the link not of this structure then open it in the standard webbrowser (behaviour as before).

---

(1) Actually I think, that rxMarkdown is even more broken. As reported in 65, the library is technically also ignoring links like [<text>](https://example.com). But since they also implemented autolinking of urls in text (<text> https://example.com <moretext>), the links are recognized in the end. But not because they are in markdown link syntax, but because the string contains http<something>.

Unfortunately I don't expect this to get fixed without forking the library (if I look at their activity)…

Conclusion

I know this replacement-logic is really not nice, but I didn't see a better approach. As long as rxMarkdown does not handle markdown links correctly, we can't influence the on-click behaviour.

If you can think of something nicer, I'd be happy to dig deeper :)

While technically, the users shouldn't realize the workaround at all, but in reality there is a fraction of a second, after the text gets replaced where the replaced links are visible to the endusers. I guess we could somehow get rid of that if you decide that the PR in in principle mergeable.

But I would also understand if you don't like the current state.

Let's see what you think and hopefully one of you has a better idea.

[lenzls]

By the way, I have experience in Java but not on Android. So forgive me if I did some things a little unconventional.
I was surprised that it actually uses a different JDK and we are stuck with some subset of jdk8. Not even streams and stuff.

Also I couldn't get your existing tests to run and figured that they were broken in the beginning. If that assumption wasn't true, I can revert my changes there.
The old ones are still not green, but at least they compile now and the newly added ones from me run through :)

[stefan-niedermann]

By the way, I have experience in Java but not on Android. So forgive me if I did some things a little unconventional.

Well i'm a web developer actually, so nothing to shame for 😆

I was surprised that it actually uses a different JDK and we are stuck with some subset of jdk8. Not even streams and stuff.

Yeah 😢 at least lambdas are available.

Okay, basically i think we can go this way, even if it might not be optimal.

in reality there is a fraction of a second, after the text gets replaced where the replaced links are visible to the endusers

I will check this with a slow virtual device, maybe we can tune the performance here a little bit, so noone will notice it.

[lenzls]

Okay, basically i think we can go this way, even if it might not be optimal.

Nice, and while working on #549 we should be able to replace the hacky part easily anyway.

I will check this with a slow virtual device, maybe we can tune the performance here a little bit, so noone will notice it.

If it helps, I had the issue on my actual phone. It is a Xiaomi A1 with Android 9.

[stefan-niedermann]

Some changes left :)