Fix 3364 editor ctrl click #3478

AWolf81 · 2020-02-19T22:13:48Z

Description

I've added the missing link handling for :tag, :line & :note links in editor (ctrl+click handling). This branch is based on branch feature-tag-links so this PR is waiting for PR #3002 to be approved/merged.

I first wanted to move the script from MarkdownPreview link handling to a file that we can use at both location but I think it's OK to have some code duplication at two locations.
It's also not identical code but pretty similar. If there is a location where I can put the specialLinkHandler method, I can add it but I think it's also OK like I have added it.

You can test the link handling with the following Markdown (requires some updates so tags & notes are available):

# Tag link test

- Item 1 [#Typescript](:tag:#Typescript)
- Item 2 [#1st tag](:tag:#1st)

# Note link
[XSS plantuml](:note:9a4c36c2-84e9-448a-a01c-eb1e555b7a6b)

# Some dummy text (for line link test)
Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.

# Line link
[Goto line 3](:line:3)

# External link
[Google](https://www.google.com)

Issue fixed

#3364

Type of changes

🔘 Bug fix (Change that fixed an issue)
⚪ Breaking change (Change that can cause existing functionality to change)
⚪ Improvement (Change that improves the code. Maybe performance or development improvement)
⚪ Feature (Change that adds new functionality)
⚪ Documentation change (Change that modifies documentation. Maybe typo fixes)

Checklist:

🔘 My code follows the project code style
⚪ I have written test for my code and it has been tested
⚪ All existing tests have been passed
🔘 I have attached a screenshot/video to visualize my change if possible

Screenshot:

Code decisions:

Created eventEmitter.emit method in hyperlink.js as importing from eventEmitter.js was not working. I think that's OK as only the emit method is needed here.
New event dispatch:push added to browser/main/main.js it's used to dispatch a push action to connected-router so it's possible to change the route for tag link handling. Not sure about the naming but I think it is OK. Location of the listener is also OK in main.js or is there a better location?
Reading the link href from the editor is a bit different compared to the MarkdownPreview. It is using the innerText of the clicked node, trims whitespaces (in case there are some - probably optional) and remove the braces.

ZeroX-DG · 2020-02-23T09:10:54Z

extra_scripts/codemirror/addon/hyperlink/hyperlink.js

+      parser.href = rawHref
+      const { href, hash } = parser
+
+      if (!rawHref) return // not checked href because parser will create file://... string for [empty link]()


Can you remove this redundant line please?

OK, no problem. I've removed it.

ZeroX-DG · 2020-03-03T01:57:18Z

extra_scripts/codemirror/addon/hyperlink/hyperlink.js

+        return
+      }
+
+      const regexIsTagLink = /^:tag:#([\w]+)$/


Can you remove the # since we decided to remove # from tag link in the previous PR?

I've removed it but there is a new issue introduced by this change - see comment below.

ZeroX-DG · 2020-03-07T08:28:58Z

@AWolf81 ping

AWolf81 · 2020-03-08T21:04:26Z

@ZeroX-DG Sorry, for not responding. I've thought your requested changes are easy to fix but there is a new issue once I've removed the # from the link and I'm not sure how to fix it.

Codemirror is changing the link handling with-out the #. In the following screenshot you can see the difference.

The first link is as we'd like to have it :tag:1st and the second is the old one with the hash :tag:#1st. You can see at the editor highlighting that Codemirror does something different - blue text vs red.
In the html markup there are several spans for the url instead of one.

I need to do more debugging for this. Do you have an idea what could cause this? An easy fix would be to use the # in tag links but I'll try to find a solution with the consistent tag-links as discussed before.

ZeroX-DG · 2020-03-09T00:40:36Z

CodeMirror used to do weird things in the markdown syntax. For example:

Random words underlined #1007

So I think it's CodeMirror Markdown parsing issue. Currently Boostnote used its' own CodeMirror mode, so we can fix this by overriding the rule in mode I think. But I'm not quite sure how to do it yet. I'll see if I can find out a way to fix this but in the mean time u are welcome to help me investigate on it.
https://github.com/BoostIO/Boostnote/blob/master/extra_scripts/codemirror/mode/bfm/bfm.js

AWolf81 · 2020-03-09T07:36:04Z

@ZeroX-DG OK, I've found the location of the problem. It's inside codemirror/mode/gfm.js in the regex

var urlRE = /^((?:(?:aaas?|about|acap|adiumxtra|af[ps]|aim|apt|attachment|aw|beshare|bitcoin|bolo|callto|cap|chrome(?:-extension)?|cid|coap|com-eventbrite-attendee|content|crid|cvs|data|dav|dict|dlna-(?:playcontainer|playsingle)|dns|doi|dtn|dvb|ed2k|facetime|feed|file|finger|fish|ftp|geo|gg|git|gizmoproject|go|gopher|gtalk|h323|hcp|https?|iax|icap|icon|im|imap|info|ipn|ipp|irc[6s]?|iris(?:\.beep|\.lwz|\.xpc|\.xpcs)?|itms|jar|javascript|jms|keyparc|lastfm|ldaps?|magnet|mailto|maps|market|message|mid|mms|ms-help|msnim|msrps?|mtqp|mumble|mupdate|mvn|news|nfs|nih?|nntp|notes|oid|opaquelocktoken|palm|paparazzi|platform|pop|pres|proxy|psyc|query|res(?:ource)?|rmi|rsync|rtmp|rtsp|secondlife|service|session|sftp|sgn|shttp|sieve|sips?|skype|sm[bs]|snmp|soap\.beeps?|soldat|spotify|ssh|steam|svn|tag|teamspeak|tel(?:net)?|tftp|things|thismessage|tip|tn3270|tv|udp|unreal|urn|ut2004|vemmi|ventrilo|view-source|webcal|wss?|wtai|wyciwyg|xcon(?:-userid)?|xfire|xmlrpc\.beeps?|xmpp|xri|ymsgr|z39\.50[rs]?):(?:\/{1,3}|[a-z0-9%])|www\d{0,3}[.]|[a-z0-9.\-]+[.][a-z]{2,4}\/)(?:[^\s()<>]|\([^\s()<>]*\))+(?:\([^\s()<>]*\)|[^\s`*!()\[\]{};:'".,<>?«»“”‘’]))/i

There is a tag matcher that's causing the problem. I have to check where I can override the regex rule in bfm.js or should we change the tag link syntax back to :tag:#tagname? Because changing the regex would limit the user from using tag inside urls.

The other issue with feedback is not happening.

During testing, I've found another issue: If your search term is matching tag then the link handling is not working as expected.

ZeroX-DG · 2020-03-10T20:23:34Z

It's tempting to switch back to the # syntax but that will conflict with @Rokt33r idea. We probably need to fix this instead of using a workaround.

AWolf81 · 2020-03-12T07:05:17Z

Yes, you're right. We should try to fix it.

I've changed the regex URL_re in gfm by removing the tag then it was working. I'll check later what link it was planned to match because I think it shouldn't match:
tag:something

Do you know how I can overwrite or replace the gfm inside the bfm mode?

ZeroX-DG · 2020-03-12T11:53:10Z

That's a hard problem, I'm still investigating how we can do that. I wonder if we create our custom gfm mode file and include it like bfm mode, would it work?

AWolf81 · 2020-03-13T22:34:54Z

@ZeroX-DG Yes, that was a hard one.
But I think it's OK now. Please have a look.

I've added gfm with a modified regex to Boostnote. I removed the tag from urlRe. Do you know what link the tag should match? Is it tag://something? I haven't found anything about it in my Google search - so I can't improve the Regex.

I've also fixed the other issue with the search term hitting a link. Just highlighting of the link looks a bit off but the click handler is working as expected. (I also added this to normal link handler - code duplication is OK here as it's only used in two locations)
Should I check the highlighting as well or is it OK as it is?

ZeroX-DG · 2020-03-14T10:35:16Z

The highlighting works flawless on my side. I think this looks cool. Can you show me the highlighting bug on your side @AWolf81 ?

BTW I have no idea with tag in URL is for 😄

AWolf81 · 2020-03-14T12:40:18Z

@ZeroX-DG I wouldn't say it's a bug. I just think this could be improved.
Hovering over the search term tag just shows ctrl+click tooltip but no link highlighting displayed (see first screenshot below). Or hovering over the other parts : or :1st will just highlight that part of the link instead of the complete link.

Hovered search term screenshot:

And here with-out a search term and correct light blue highlighting:

I'm not sure if I'm a bit too critical here but fixing this could be a bit tricky as the yellow highlighting is conflicting the hover effect and that there a multiple spans involved. I think that could be also fixed later as the click handling is working as expected.

ZeroX-DG · 2020-03-14T21:44:19Z

@AWolf81 I see. I think it's gonna be very tricky to fix the highlighting for the search term. Since it's not a major issue, I don't think with should hold this PR and fix it. I think for now this PR looks great, I'll approve it and get it merge first. Then we can find a way to deal with that search term highlighting stuff.

ZeroX-DG · 2020-03-14T21:51:28Z

A possible solution for that issue is by using markText to create our own link element
https://codemirror.net/doc/manual.html#markText

ZeroX-DG

LGTM 🎉

Flexo013 · 2020-03-27T13:32:59Z

Nice fix! This works properly for both internal links (to different views) and external links.

However, there does still seem to be an issue with this not working for references within a single note. For example in the TOC:

add special link handling

f659c77

ZeroX-DG added the awaiting review ❇️ Pull request is awaiting a review. label Feb 19, 2020

ZeroX-DG requested changes Mar 3, 2020

View reviewed changes

ZeroX-DG added awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. and removed awaiting review ❇️ Pull request is awaiting a review. labels Mar 3, 2020

AWolf81 added 2 commits March 3, 2020 07:29

address requested changes - tag link & redundant line

b155126

Merge branch 'master' into fix-3364-editor-ctrl-click

f5ecdc3

AWolf81 added 2 commits March 8, 2020 21:59

WIP: Change space before parens. Tag link handling issue present.

32c0005

Remove duplicated if

905a13a

add gfm with modified regex & improve link text handling

7c707ac

Flexo013 added awaiting review ❇️ Pull request is awaiting a review. and removed awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. labels Mar 13, 2020

ZeroX-DG approved these changes Mar 14, 2020

View reviewed changes

ZeroX-DG added approved 👍 Pull request has been approved by sufficient reviewers. and removed awaiting review ❇️ Pull request is awaiting a review. labels Mar 14, 2020

Rokt33r added this to the v0.15.2 milestone Mar 25, 2020

Rokt33r merged commit 791abab into BoostIO:master Mar 25, 2020

Uh oh!

Fix 3364 editor ctrl click #3478

Fix 3364 editor ctrl click #3478

Uh oh!

Conversation

AWolf81 commented Feb 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issue fixed

Type of changes

Checklist:

Screenshot:

Code decisions:

Uh oh!

ZeroX-DG Feb 23, 2020

Choose a reason for hiding this comment

Uh oh!

AWolf81 Mar 8, 2020

Choose a reason for hiding this comment

Uh oh!

ZeroX-DG Mar 3, 2020

Choose a reason for hiding this comment

Uh oh!

AWolf81 Mar 8, 2020

Choose a reason for hiding this comment

Uh oh!

ZeroX-DG commented Mar 7, 2020

Uh oh!

AWolf81 commented Mar 8, 2020

Uh oh!

ZeroX-DG commented Mar 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AWolf81 commented Mar 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZeroX-DG commented Mar 10, 2020

Uh oh!

AWolf81 commented Mar 12, 2020

Uh oh!

ZeroX-DG commented Mar 12, 2020

Uh oh!

AWolf81 commented Mar 13, 2020

Uh oh!

ZeroX-DG commented Mar 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AWolf81 commented Mar 14, 2020

Uh oh!

ZeroX-DG commented Mar 14, 2020

Uh oh!

ZeroX-DG commented Mar 14, 2020

Uh oh!

ZeroX-DG left a comment

Choose a reason for hiding this comment

Uh oh!

Flexo013 commented Mar 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AWolf81 commented Feb 19, 2020 •

edited

Loading

ZeroX-DG commented Mar 9, 2020 •

edited

Loading

AWolf81 commented Mar 9, 2020 •

edited

Loading

ZeroX-DG commented Mar 14, 2020 •

edited

Loading