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

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

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@
- #3007: Fix links becoming text when a message is edited
- #3018: Fix MUC icons not functioning.

## 9.1.2 (2022-05-15)
- Added fixes for Issue #2857. The fix is regarding Converse.js incorrectly applying text styling effects to HTTP URLs that contain special characters(`_`, `\``, `\`\`\``, `~`, `*`). With the fixes applied, the hyperlinks encompass the entire HTTP URL instead of only part of it
- Added integration test for the fixes above to the following file: `/src/plugins/chatview/tests/chatbox.js/`

## 9.1.1 (2022-05-05)

- GIFs don't render inside unfurls and cause a TypeError
Expand Down
41 changes: 41 additions & 0 deletions src/plugins/chatview/tests/chatbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,45 @@ describe("Chatboxes", function () {
}));
});
});

describe("Chatbox Message Styling", function() {

it("is not applied when sending a chat message containing a HTTP URL with styling template characters( _, \`, \`\`\`, ~, *) in the middle of it",
mock.initConverse(['chatBoxesFetched'], {}, async function (_converse) {

await mock.waitForRoster(_converse, 'current');
await mock.openControlBox(_converse);
const contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@montague.lit';

spyOn(_converse.api, "trigger").and.callThrough();
await mock.openChatBoxFor(_converse, contact_jid);
const view = _converse.chatboxviews.get(contact_jid);
let message = 'https://nitter.kavin.rocks/_MG_/~/*/1506109152665382920';
await mock.sendMessage(view, message);

expect(view.model.messages.length === 1).toBeTruthy();
const stored_messages = await view.model.messages.browserStorage.findAll();
expect(view.model.messages.models[0].attributes.message).toEqual(message);
expect(view.model.messages.length).toBe(1);

// try second URL
let message2 = 'https://nitter.george.rocks/_G*_/\`/\`\`\`/665382920';
await mock.sendMessage(view, message2);

expect(view.model.messages.length === 2).toBeTruthy();
expect(view.model.messages.models[1].attributes.message).toEqual(message2);
expect(view.model.messages.length).toBe(2);

// try third URL
let message3 = 'https://nitter.frank.rocks/_OP_/_NP_/\`\`\`/*/~/_qweq_665382920';
await mock.sendMessage(view, message3);

expect(view.model.messages.length === 3).toBeTruthy();
expect(view.model.messages.models[2].attributes.message).toEqual(message3);
expect(view.model.messages.length).toBe(3);
}));

});;
});

describe("Special Messages", function () {
Expand Down Expand Up @@ -1052,5 +1091,7 @@ describe("Chatboxes", function () {
await mock.openChatBoxFor(_converse, sender_jid);
expect(select_msgs_indicator().textContent).toBe('1');
}));


});
});
136 changes: 122 additions & 14 deletions src/shared/rich-text.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ export class RichText extends String {
const full_offset = local_offset + this.offset;
const urls_meta = this.media_urls || getMediaURLsMetadata(text, local_offset).media_urls || [];
const media_urls = getMediaURLs(urls_meta, text, full_offset);


media_urls.filter(o => !o.is_encrypted).forEach(url_obj => {
const url_text = url_obj.url;
Expand Down Expand Up @@ -209,22 +210,112 @@ export class RichText extends String {
}

/**
* Look for XEP-0393 styling directives and add templates for rendering them.

* Check for whether a number is within multiple ranges of begin/end references.
* For example: given @param { Array } references array of [{ begin: 23, end: 30, ... }, { begin: 80, end: 100, ... }], function will return false if @param { Integer } search_range = `50`. If @param { Integer } search_range = `90`, function will return true
* @param { Array } references - An array containing reference objects({begin: 1, end: 2, template: {...} }) with ranges of numbers representing begin and end portions of outgoing Chat messages
* @param { Integer } search_range - This value will be compared to all the ranges contained in @param { Array } references in order to determine whether it lying inside any of those ranges
*/
addStyling () {
if (!containsDirectives(this, this.mentions)) {
return;
checkNumInRange(references, search_range){
let v = [];

let map = new Map();

for(var i = 0; i < references.length; i++){
v.push(references[i].start);
map.set(references[i].start, 1);
v.push(references[i].end);
map.set(references[i].end, 2);
}

v.sort(function(a, b){ return a - b; });
let index = this.lowerBound(v, search_range);

if(index >= 0 && v[index] == search_range){
return true;
}else{
if(index >= 0 && map.get(v[index]) == 2){
return true;
}else{
return false;
}
}

}
/**
* Used to obtain the nearest lower bound value for a given @param { Integer } value
* For example: given @param { Array } array contaiing [23, 30, 80, 100], a @param { Integer } value = 90 would return 80
* @param { Array } array - An array containing a series a integers which are derived from @param { Array } references in `checkNumInRange(references, search_range)`
* @param { Integer } value - This is the search term from which a lower bound value nearest to it will be returned by the function
*/
lowerBound(array, value){
let low = 0;
let high = array.length;
while(low < high){
let mid = Math.floor((low + high) / 2);
if(value <= array[mid]){
high = mid;
}else{
low = mid + 1;
}
}
return low;
}


/**
* Look for XEP-0393 styling directives and add templates for rendering
* them.
*/
addStyling () {
this.addAnnotations(this.addHyperlinks);

const references = [];
const mention_ranges = this.mentions.map(m =>
Array.from({ 'length': Number(m.end) }, (_, i) => Number(m.begin) + i)
);
let i = 0;
while (i < this.length) {
if (mention_ranges.filter(r => r.includes(i)).length) { // eslint-disable-line no-loop-func
// Don't treat potential directives if they fall within a
// declared XEP-0372 reference
var filtered_refs = [];
var urls_coords = [];

for(var i = 0; i < this.references.length; i++){
var ending_string = this.references[i].template.strings.length;
if(this.references[i].template.strings[ending_string - 1] == "</a>"){
var new_coords = {
start: this.references[i].begin,
end: this.references[i].end
};
urls_coords.push(new_coords);
}
}
if (containsDirectives(this, this.mentions)) {
const mention_ranges = this.mentions.map(m =>
Array.from({ 'length': Number(m.end) }, (v, i) => Number(m.begin) + i)
);
let i = 0;
while (i < this.length) {
if (mention_ranges.filter(r => r.includes(i)).length) { // eslint-disable-line no-loop-func
// Don't treat potential directives if they fall within a
// declared XEP-0372 reference
i++;
continue;
}
const { d, length } = getDirectiveAndLength(this, i);
if (d && length) {
const is_quote = isQuoteDirective(d);
const end = i + length;
const slice_end = is_quote ? end : end - d.length;
let slice_begin = d === '```' ? i + d.length + 1 : i + d.length;
if (is_quote && this[slice_begin] === ' ') {
// Trim leading space inside codeblock
slice_begin += 1;
}
const offset = slice_begin;
const text = this.slice(slice_begin, slice_end);
references.push({
'begin': i,
'template': getDirectiveTemplate(d, text, offset, this.options),
end
});
i = end;
}

i++;
continue;
}
Expand All @@ -247,11 +338,27 @@ export class RichText extends String {
});
i = end;
}
i++;

if(urls_coords.length > 0){
for(var k = 0; k < references.length; k++){
var start_range = this.checkNumInRange(urls_coords, references[k].begin);
var end_range = this.checkNumInRange(urls_coords, references[k].end);

if(!start_range && !end_range){
filtered_refs.push(references[k]);
}
}
}else{
filtered_refs = references;
}
const begin_end_coords = filtered_refs.map(ref => ref.begin);
filtered_refs = filtered_refs.filter(({begin}, index) => !begin_end_coords.includes(begin, index + 1));

}
references.forEach(ref => this.addTemplateResult(ref.begin, ref.end, ref.template));
filtered_refs.forEach(ref => this.addTemplateResult(ref.begin, ref.end, ref.template));
}


trimMeMessage () {
if (this.offset === 0) {
// Subtract `/me ` from 3rd person messages
Expand Down Expand Up @@ -301,6 +408,7 @@ export class RichText extends String {
*/
await api.trigger('beforeMessageBodyTransformed', this, { 'Synchronous': true });


this.render_styling && this.addStyling();
this.addAnnotations(this.addMentions);
this.addAnnotations(this.addHyperlinks);
Expand Down