zulip / zulip

Zulip server and web application. Open-source team chat that helps teams stay productive and focused.
https://zulip.com
Apache License 2.0
21.33k stars 7.74k forks source link

Fix clicking on links where text is a code block #11529

Open timabbott opened 5 years ago

timabbott commented 5 years ago

Apparently, if you do this syntax, e.g.:

Opened [Update TypeScript infrastructure and migrate dict.js](#11513) for review.

We generate a link formatted as a code block, which is nice, but clicking the link just opens compose, probably due to a click handler ordering/precedence issue. See for example:

https://chat.zulip.org/#narrow/stream/65-checkins/topic/Tommy.20Ip/near/697676

This may be a bit tricky to fix, since debugging this sort of click handler ordering issue can be challenging.

kunal-mohta commented 5 years ago

@timabbott I don't think there is any bug here, as this worked for me (see gif below). I guess why it didn't work in the thread that you have linked here is because of the incorrect link provided in the parathesis. Try a valid link like Opened [Zulip github](https://github.com/zulip/zulip) for review. ezgif-5-6ef6a82b6ef8

timabbott commented 5 years ago

Try Chrome?

tommyip commented 5 years ago

The link didn't get linkified screenshot from 2019-02-13 22-11-19

kunal-mohta commented 5 years ago

Try Chrome?

Worked for me there too. ezgif-3-18a47788ea19

rishig commented 5 years ago

hm, working for me on Chrome as well. (Though it sounds like the original issue is not browser-specific?)

rishig commented 5 years ago

ok, I reproed. The link has to come from a linkification filter. (Kunal, let me know if that doesn't make sense.)

kunal-mohta commented 5 years ago

@rishig Yes, a brief explanation would be helpful. :sweat_smile: How did you reproduce the issue?

timabbott commented 5 years ago

@kunal-mohta you need to setup a "Linkifier" in organization settings, and then generate a link like that. You can get the source content for Tommy's message and copy the relevant Linkifier from chat.zulip.org's organization settings to set this up in your development environment.

rishig commented 5 years ago

I should have linked to https://zulipchat.com/help/add-a-custom-linkification-filter.

vrongmeal commented 5 years ago

The issue is not specific to links with text as code blocks, it is purely related to the linkifier. Even with simple text, syntax like:

[some text here](#12345)

points to http://localhost:9991/#12345

[edit] The compose box opens maybe because the click propagates to the code block, simply clicking the link doesn't open the compose box.

tommyip commented 5 years ago

The problem is the realm filter is expanded before the link, so at some point in the renderer the markdown is transformed into

[some text](<a href="https://github.com/zulip/zulip/pull/123" target="_blank" title="https://github.com/zulip/zulip/pull/123">#123</a>)

@zulipbot claim

timabbott commented 5 years ago

@aero31aero FYI since this is a markdown processor ordering bug.

aero31aero commented 5 years ago

Interesting. I believe this was present before the recent changes to ordering as well. It shouldn't be too hard to fix.

@zulipbot claim

Edit: just saw Tommy has claimed it.

zulipbot commented 5 years ago

Hello @aero31aero, it looks like someone has already claimed this issue! Since we believe multiple assignments to the same issue may cause some confusion, we encourage you to search for other unclaimed issues to work on. However, you can always reclaim this issue if no one is working on it.

We look forward to your valuable contributions!

timabbott commented 5 years ago

You can chat with Tommy and collaborate -- he might benefit from your context.

tommyip commented 5 years ago

Ok so I fixed the frontend markdown. My solution is a bit of a hack :grimacing: since I got round the ordering problem by replacing the realm filter with a marker before rendering and inserting the link afterwards.

@aero31aero would you mind reviewing my PR?

zulipbot commented 5 years ago

Hello @tommyip, you have been unassigned from this issue because you have not updated this issue or any referenced pull requests for over 14 days.

You can reclaim this issue or claim any other issue by commenting @zulipbot claim on that issue.

Thanks for your contributions, and hope to see you again soon!