Fixes for Issue #2857: Text Styling Should Not Be Applied In The Middle Of URLs

[ShehanAT]

Description:

This fix is regarding Converse.js incorrectly applying text styling effects to HTTP URLs that contain special characters( _, `, ```, ~, *).

Previously the hyperlink in the sent chat message would end prematurely due to the special characters contained in the HTTP URL. Now, the hyperlink in the sent chat message encompass the whole HTTP URL regardless of the presence of special characters.

Here is a screenshot of the chat window after applying the fix:

Fixes #2857

Testing:

I added a new passing integration test in the following path: /src/plugins/chatview/tests/chatbox.js
Test name: Chatbox Message Styling > is not applied when sending a chat message containing a HTTP URL with styling template character in the middle of it

I have run make check which resulted in all 507 tests passing.

Conditions:

I have ensured that the following conditions are met:

• Add a changelog entry for your change in CHANGES.md
• When adding a configuration variable, please make sure to
  document it in docs/source/configuration.rst
• Please add a test for your change. Tests can be run in the commandline
  with make check or you can run them in the browser by running make serve
  and then opening http://localhost:8000/tests.html.

[lgtm-com]

This pull request introduces 1 alert when merging 19f25bc into 858a605 - view on LGTM.com

new alerts:

• 1 for Expression has no effect

[jcbrand]

Thanks for taking the time and making this pull request.

The above fix isn't sufficient because the text is not guaranteed to be only a URL. There could be URLs anywhere in the text, not just at the start.

So for example AFAICT your fix wouldn't work on this text:

Here is the link I wanted to send you: https://nitter.kavin.rocks/_MG_/status/1506109152665382920

It's great that you're adding tests, so now you can add a test for the message above, check whether it passes and if it doesn't you can improve the code until it does.

For a better solution, take a look at how @ mentions are ignored when looking for styled text.
See here:

converse.js/src/shared/rich-text.js

Line 218 in 1ad6de2

const mention_ranges = this.mentions.map(m =>

Similar to how the mention_ranges array is used to avoid styling mentions, we need to know where the URLs are so that we can avoid them as well.

Looks like the urls_meta array has the necessary information (similarly to mention_ranges).
See here:

converse.js/src/shared/rich-text.js

Line 127 in 1ad6de2

const urls_meta = this.media_urls || getMediaURLsMetadata(text, local_offset).media_urls || [];

So, in addStyling, you can compute urls_meta and then use that to avoid styling URLs, similarly to how we avoid styling mentions.

You can probably even combine both URLs and mentions into one array of ranges of text which shouldn't be styled.

[ShehanAT]

Ok I see what you mean.
Do you happen to know where this.model.get('references') is being populated?
I ask because this.mentions is populated by this.model.get('references') as seen in src/shared/chat/message-body.js line 48 but I'm not finding where the references field is being populated...

If I can find out how references are being identified, I can make a similar implementation for HTTP Urls

[jcbrand]

For incoming messages, the references are extracted when the stanza is parsed:

converse.js/src/headless/plugins/muc/parsers.js

Line 225 in a228cf2

'references': getReferences(stanza),

For outgoing messages it's done somewhere else, I'm not sure where right now. However, I don't think this is relevant to fixing this bug.

The getMediaURLsMetadata function does the work for you of getting the URLs and you can call that function inside addStyling in order to get a list of URLs to ignore.

[lgtm-com]

This pull request introduces 1 alert when merging fc844dc into 8dc8b1d - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

[ShehanAT]

I made a potential fix based on your advise @jcbrand .

Now, the text styling is not being applied when there is a URL in the middle of the chat message:

I've ran make check which finished with 3 failing tests.

This potential fix is pretty messy readability wise, but could you take a look and let me know what you think?

Thanks.

[lgtm-com]

This pull request introduces 1 alert and fixes 1 when merging f481471 into 5760379 - view on LGTM.com

new alerts:

• 1 for Superfluous trailing arguments

fixed alerts:

• 1 for Superfluous trailing arguments