alert_words: Don't muck up rendered HTML content while munging.

Prior to this we were also performing highlighting inside HTML tags
which was wrong and causing weird behavior. Like, for example, if
someone added `emoji` as an alert word then any message containing
both emoji and alert word was rendered with a jumbo emoji.

Fixes: #4357.
This commit is contained in:
Harshit Bansal
2017-07-29 19:17:12 +00:00
committed by Tim Abbott
parent 4a5f82bc71
commit a13535ff1f
2 changed files with 17 additions and 7 deletions

View File

@@ -3,7 +3,7 @@ add_dependencies({
});
set_global('page_params', {
alert_words: ['alertone', 'alerttwo', 'alertthree', 'al*rt.*s', '.+'],
alert_words: ['alertone', 'alerttwo', 'alertthree', 'al*rt.*s', '.+', 'emoji'],
});
set_global('feature_flags', {
@@ -41,7 +41,9 @@ var question_word_message = { sender_email: 'another@zulip.com', content: '<p>st
var alert_domain_message = { sender_email: 'another@zulip.com', content: '<p>now with link <a href="http://www.alerttwo.us/foo/bar" target="_blank" title="http://www.alerttwo.us/foo/bar">www.alerttwo.us/foo/bar</a></p>',
alerted: true };
// This test ensure we are not mucking up rendered HTML content.
var message_with_emoji = { sender_email: 'another@zulip.com', content: '<p>I <img alt=":heart:" class="emoji" src="/static/generated/emoji/images/emoji/unicode/2764.png" title="heart"> emoji!</p>',
alerted: true };
(function test_notifications() {
assert(!alert_words.notifies(regular_message));
@@ -52,6 +54,7 @@ var alert_domain_message = { sender_email: 'another@zulip.com', content: '<p>now
assert(alert_words.notifies(multialert_message));
assert(alert_words.notifies(unsafe_word_message));
assert(alert_words.notifies(alert_domain_message));
assert(alert_words.notifies(message_with_emoji));
}());
(function test_munging() {
@@ -82,4 +85,7 @@ var alert_domain_message = { sender_email: 'another@zulip.com', content: '<p>now
alert_words.process_message(alert_domain_message);
assert.equal(alert_domain_message.content, '<p>now with link <a href="http://www.alerttwo.us/foo/bar" target="_blank" title="http://www.alerttwo.us/foo/bar">www.<span class=\'alert-word\'>alerttwo</span>.us/foo/bar</a></p>');
alert_words.process_message(message_with_emoji);
assert.equal(message_with_emoji.content, '<p>I <img alt=":heart:" class="emoji" src="/static/generated/emoji/images/emoji/unicode/2764.png" title="heart"> <span class=\'alert-word\'>emoji</span>!</p>');
}());

View File

@@ -13,9 +13,6 @@ function escape_user_regex(value) {
return value.replace(/[\-\[\]{}()*+?.,\\\^$|#\s]/g, "\\$&");
}
var find_href_backwards = /href=['"][\w:\/\.]+$/;
var find_title_backwards = /title=['"][\w:\/\.]+$/;
exports.process_message = function (message) {
if (!exports.notifies(message)) {
return;
@@ -32,9 +29,16 @@ exports.process_message = function (message) {
'(' + after_punctuation + ')' , 'ig');
message.content = message.content.replace(regex, function (match, before, word,
after, offset, content) {
// Don't munge URL hrefs
// Logic for ensuring that we don't muck up rendered HTML.
var pre_match = content.substring(0, offset);
if (find_href_backwards.exec(pre_match) || find_title_backwards.exec(pre_match)) {
// We want to find the position of the `<` and `>` only in the
// match and the string before it. So, don't include the last
// character of match in `check_string`. This covers the corner
// case when there is an alert word just before `<` or `>`.
var check_string = pre_match + match.substring(0, match.length - 1);
var in_tag = check_string.lastIndexOf('<') > check_string.lastIndexOf('>');
// Matched word is inside a HTML tag so don't perform any highlighting.
if (in_tag === true) {
return before + word + after;
}
return before + "<span class='alert-word'>" + word + "</span>" + after;