-
-
Notifications
You must be signed in to change notification settings - Fork 830
Replies #1741
Replies #1741
Changes from 43 commits
1c3d8cb
a15d024
1814546
fbb950e
23c0daa
7425d01
25bc9cf
7bf05b0
aeedf48
2e3cbb3
8062494
cb293a8
f1a3592
0d6fc9b
4c08c51
8fa56f8
6510989
34b427d
7048d85
90f9bad
665ddcc
df56a67
3b02766
ed8f087
023632e
14f29e4
0ed3563
6610734
b5ed08e
fac89d9
a390cec
8fa4a52
80fbc75
9c2e3e2
2854f2b
1d90835
9df2f7d
4081321
871fee2
f765db7
c77807b
8b1e411
a90bd6c
58cd585
06408f8
563fc9a
7e8288f
941bb94
d680d80
057815a
5f920f7
0f11bc6
71acf87
3de679b
82d1179
4a0a5c6
97fecae
88f4891
3ba9f56
fdf63fd
ca766df
3050553
68dd57f
2e29a08
bbf4d3e
5bb15b1
407be88
c00c52e
4021fc0
41af9f7
bbce6ee
f0bd4a5
11ae080
db55f87
ec4ec47
f2102e2
4c3f811
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,8 @@ limitations under the License. | |
|
||
'use strict'; | ||
|
||
import ReplyThread from "./components/views/elements/ReplyThread"; | ||
|
||
const React = require('react'); | ||
const sanitizeHtml = require('sanitize-html'); | ||
const highlight = require('highlight.js'); | ||
|
@@ -184,6 +186,7 @@ const sanitizeHtmlParams = { | |
], | ||
allowedAttributes: { | ||
// custom ones first: | ||
blockquote: ['data-mx-reply'], // for reply fallback | ||
font: ['color', 'data-mx-bg-color', 'data-mx-color', 'style'], // custom to matrix | ||
span: ['data-mx-bg-color', 'data-mx-color', 'style'], // custom to matrix | ||
a: ['href', 'name', 'target', 'rel'], // remote target: custom to matrix | ||
|
@@ -408,12 +411,14 @@ class TextHighlighter extends BaseHighlighter { | |
* | ||
* opts.highlightLink: optional href to add to highlighted words | ||
* opts.disableBigEmoji: optional argument to disable the big emoji class. | ||
* opts.stripReplyFallback: optional argument specifying the event is a reply and so fallback needs removing | ||
*/ | ||
export function bodyToHtml(content, highlights, opts={}) { | ||
let isHtml = (content.format === "org.matrix.custom.html"); | ||
let isHtml = content.format === "org.matrix.custom.html" && content.formatted_body; | ||
|
||
let bodyHasEmoji = false; | ||
|
||
let strippedBody; | ||
let safeBody; | ||
// XXX: We sanitize the HTML whilst also highlighting its text nodes, to avoid accidentally trying | ||
// to highlight HTML tags themselves. However, this does mean that we don't highlight textnodes which | ||
|
@@ -431,17 +436,22 @@ export function bodyToHtml(content, highlights, opts={}) { | |
}; | ||
} | ||
|
||
bodyHasEmoji = containsEmoji(isHtml ? content.formatted_body : content.body); | ||
let formattedBody = content.formatted_body; | ||
if (opts.stripReplyFallback && formattedBody) formattedBody = ReplyThread.stripHTMLReply(formattedBody); | ||
strippedBody = opts.stripReplyFallback ? ReplyThread.stripPlainReply(content.body) : content.body; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm really not sure that this should be part of sanitising. The quoted text does not threaten the security of the app. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure where better to put this, technically this method is not specific to sanitising, its just There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, you're totally right. |
||
|
||
bodyHasEmoji = containsEmoji(isHtml ? formattedBody : content.body); | ||
|
||
|
||
// Only generate safeBody if the message was sent as org.matrix.custom.html | ||
if (isHtml) { | ||
safeBody = sanitizeHtml(content.formatted_body, sanitizeHtmlParams); | ||
safeBody = sanitizeHtml(formattedBody, sanitizeHtmlParams); | ||
} else { | ||
// ... or if there are emoji, which we insert as HTML alongside the | ||
// escaped plaintext body. | ||
if (bodyHasEmoji) { | ||
isHtml = true; | ||
safeBody = sanitizeHtml(escape(content.body), sanitizeHtmlParams); | ||
safeBody = sanitizeHtml(escape(strippedBody), sanitizeHtmlParams); | ||
} | ||
} | ||
|
||
|
@@ -458,7 +468,7 @@ export function bodyToHtml(content, highlights, opts={}) { | |
let emojiBody = false; | ||
if (!opts.disableBigEmoji && bodyHasEmoji) { | ||
EMOJI_REGEX.lastIndex = 0; | ||
const contentBodyTrimmed = content.body !== undefined ? content.body.trim() : ''; | ||
const contentBodyTrimmed = strippedBody !== undefined ? strippedBody.trim() : ''; | ||
const match = EMOJI_REGEX.exec(contentBodyTrimmed); | ||
emojiBody = match && match[0] && match[0].length === contentBodyTrimmed.length; | ||
} | ||
|
@@ -469,9 +479,11 @@ export function bodyToHtml(content, highlights, opts={}) { | |
'markdown-body': isHtml, | ||
}); | ||
|
||
return isHtml ? | ||
<span className={className} dangerouslySetInnerHTML={{ __html: safeBody }} dir="auto" /> : | ||
<span className={className} dir="auto">{ content.body }</span>; | ||
if (isHtml) { | ||
return <span className={className} dangerouslySetInnerHTML={{ __html: safeBody }} dir="auto" />; | ||
} | ||
|
||
return <span className={className} dir="auto">{ strippedBody }</span>; | ||
} | ||
|
||
export function emojifyText(text) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,6 +45,7 @@ import { KeyCode, isOnlyCtrlOrCmdKeyEvent } from '../../Keyboard'; | |
import RoomViewStore from '../../stores/RoomViewStore'; | ||
import RoomScrollStateStore from '../../stores/RoomScrollStateStore'; | ||
import SettingsStore from "../../settings/SettingsStore"; | ||
import Reply from "../views/elements/ReplyThread"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This import seems unused |
||
|
||
const DEBUG = false; | ||
let debuglog = function() {}; | ||
|
@@ -916,9 +917,13 @@ module.exports = React.createClass({ | |
|
||
ContentMessages.sendContentToRoom( | ||
file, this.state.room.roomId, MatrixClientPeg.get(), | ||
).catch((error) => { | ||
).done(() => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "any unhandled rejection that ends up here will ... be thrown as an error" (see http://bluebirdjs.com/docs/api/done.html) this should be catch(...).then(...) |
||
dis.dispatch({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps a comment to explain why this dispatch is needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was for when file replies were allowed, but is still useful as it means the "Check if alone in room" stuff gets hit by file messages and not only text messages, seems kind of self-explanatory in that if we send a message, we tell the app we just did so, just like we do with textual messages? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd much prefer a comment to answer the question "why is this dispatch sent". |
||
action: 'message_sent', | ||
}); | ||
}, (error) => { | ||
if (error.name === "UnknownDeviceError") { | ||
// Let the staus bar handle this | ||
// Let the status bar handle this | ||
return; | ||
} | ||
const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog"); | ||
|
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a better explanation. What values could this attribute hold? How is it used?