Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

URL link in search broken #18770

Closed
reetp opened this issue Sep 2, 2020 · 13 comments · Fixed by #28001
Closed

URL link in search broken #18770

reetp opened this issue Sep 2, 2020 · 13 comments · Fixed by #28001
Assignees
Labels
feat: message stat: triaged Issue reviewed and properly tagged type: bug ui/ux
Milestone

Comments

@reetp
Copy link

reetp commented Sep 2, 2020

Description:

URL link in search broken if URL is wrapped (?)
(Also some weird artifacts when searching)

Steps to reproduce

Click Search
Search for expression
See URL
Click

URL like this:

http://eurospas1.com/covers-eurospas.php

Becomes this when clicked. See images below.

http://eurospas1.com/<mark>cover</mark>s-eurospas.php

Expected behavior:

Click link and go to the correct site

Actual behavior:

Note here it finds 'cover', but does NOT find 'face' - either in the Emoji, or in the URL. It does find facemask.

Screenshot_2020-09-02_17-08-18

Screenshot_2020-09-02_16-50-29

Server Setup Information:

  • Version of Rocket.Chat Server: Noticed on 3.4.x - tested on open.rocket.chat - presume 3.6.x currently
  • Operating System: Various
  • Deployment Method: Whatever open rocket runs on
  • Number of Running Instances: As above
  • DB Replicaset Oplog: As above
  • NodeJS Version: As above
  • MongoDB Version: As above

Client Setup Information

  • Desktop App or Browser Version: Chromium version 84.0.4147.105 (Official Build) Built on Ubuntu(64-bit)
  • Operating System: xUbuntu 18.04

Additional context

Also affects other searches - note how it mangles things here:

Screenshot_2020-09-02_17-06-03

If I search for facemask (and it has to be exact - just 'face' will not find the URL but does find the emoji above) note the URL when clicked does this:

https://www.uk-personalprotectionequipment.co.uk/en14683-type1-<mark>facemask</mark>

Relevant logs:

N/A

@ggazzo ggazzo added this to the 3.7.0 milestone Sep 2, 2020
@ghost
Copy link

ghost commented Sep 3, 2020

see #18696 and #18456 😅

commit adding this #16166

@reetp
Copy link
Author

reetp commented Sep 3, 2020

Damn - I did search too :-(

So this is in part a dupe of those.

However, there is a separate issue regarding search itself - I am not sure whether that gets resolved by fixing the issue.

@TBG-FR
Copy link
Contributor

TBG-FR commented Sep 7, 2020

I've been able to reproduce the issue.

I'm trying to fix this, especially using better RegEx rules. I managed to find one to exclude emojis, but handling HTML and/or URLs is a bit more complex... I'll make a pull request referencing all these issues once I'll have something interesting to test/review ;)

@ghost
Copy link

ghost commented Sep 8, 2020

image

@rocket.cat: mentions break too.


maybe you can just move https://github.com/RocketChat/Rocket.Chat/pull/16166/files#diff-137696e1986a33ce14c42062296a4484R45 up directly under the definitions? Before the msg gets rendered?

@TBG-FR
Copy link
Contributor

TBG-FR commented Sep 15, 2020

I created a PR draft, after being able to fix URLs and emojis. Still some points are needing more work or investigation

image

@rocket.cat: mentions break too.

maybe you can just move https://github.com/RocketChat/Rocket.Chat/pull/16166/files#diff-137696e1986a33ce14c42062296a4484R45 up directly under the definitions? Before the msg gets rendered?

I didn't manage to understand or reproduce any issue with mentions, can you explain better ?

@ghost
Copy link

ghost commented Sep 15, 2020

send a message with a mention @rocket.cat.
search for a letter in the name, like r. It won't show the mention as a message, but what the screenshot shows

a

@TBG-FR
Copy link
Contributor

TBG-FR commented Sep 15, 2020

Do you have the latest RocketChat version ? I tried with @all before, and now with @rocket.cat and I get no search result with a or r... Strange, maybe they modified so mentions are not searched ?

@ghost
Copy link

ghost commented Sep 15, 2020

version 3.5.2 and @all doesn't seem to trigger it. I think having no search result is not ideal.
Code seems unchanged.

If you can't reproduce, forget it 😉

@TBG-FR
Copy link
Contributor

TBG-FR commented Sep 15, 2020

Alright. If I find something I'll tell you. Anyway, considering what I see on your screenshot, adding a bit (rname to href|title) in my regex could be sufficient, we'll see.

Do you have any clue about the fact that some search result don't show up ?
I mean, if I search face, I will find messages with face-book.com but not with facebook.com, or face-mask but not facemask
It's not related to the highlighting regex, it's something that is handled before, but I can't really locate where...

@ghost
Copy link

ghost commented Sep 15, 2020

Do you have any clue about the fact that some search result don't show up ?

sorry, I am not familiar with this code base.

Maybe ask @ashwaniYDV who implemented this feature. As a member he should be able to help you even more :)

@TBG-FR
Copy link
Contributor

TBG-FR commented Sep 16, 2020

sorry, I am not familiar with this code base.

Maybe ask @ashwaniYDV who implemented this feature. As a member he should be able to help you even more :)

From what I understand, he implemented the highlighting feature, and the issue here is more related to the search feature itself (highlighting code part isn't even called, it's like these results are not found at all)... But yeah, maybe someone knowing the code base could help, thanks 😄

@TBG-FR
Copy link
Contributor

TBG-FR commented Nov 9, 2020

Still waiting on answer (about missing results) on #18913 and then I'll be able to finish the pull request and put it to review, it'll fix issues we discussed here

@ghost
Copy link

ghost commented Nov 9, 2020

Rocket.chat is so buggy, this one bug does not change much. They don't even want to help you fixing problems

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: message stat: triaged Issue reviewed and properly tagged type: bug ui/ux
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants