From 3753e5261dda23063314f4e677448c29a910a7dc Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 23 Jul 2019 09:12:24 +0200 Subject: [PATCH] Implement diffing html messages in the edit history --- package.json | 1 + .../dialogs/_MessageEditHistoryDialog.scss | 4 +- src/HtmlUtils.js | 37 ++- .../views/messages/EditHistoryMessage.js | 27 +- src/editor/deserialize.js | 16 +- src/utils/MessageDiffUtils.js | 258 ++++++++++++++++++ yarn.lock | 47 +++- 7 files changed, 345 insertions(+), 45 deletions(-) create mode 100644 src/utils/MessageDiffUtils.js diff --git a/package.json b/package.json index af7bbb7d5b2..4b73fff6a51 100644 --- a/package.json +++ b/package.json @@ -66,6 +66,7 @@ "classnames": "^2.1.2", "commonmark": "^0.28.1", "counterpart": "^0.18.0", + "diff-dom": "^4.1.3", "diff-match-patch": "^1.0.4", "emojibase-data": "^4.0.0", "emojibase-regex": "^3.0.0", diff --git a/res/css/views/dialogs/_MessageEditHistoryDialog.scss b/res/css/views/dialogs/_MessageEditHistoryDialog.scss index 3f54cc2e821..367df57e1bd 100644 --- a/res/css/views/dialogs/_MessageEditHistoryDialog.scss +++ b/res/css/views/dialogs/_MessageEditHistoryDialog.scss @@ -43,12 +43,12 @@ limitations under the License. padding: 0px 2px; } - span.mx_EditHistoryMessage_deletion { + .mx_EditHistoryMessage_deletion { color: rgb(255, 76, 85); background-color: rgba(255, 76, 85, 0.1); } - span.mx_EditHistoryMessage_insertion { + .mx_EditHistoryMessage_insertion { color: rgb(26, 169, 123); background-color: rgba(26, 169, 123, 0.1); } diff --git a/src/HtmlUtils.js b/src/HtmlUtils.js index fb7a14d6fb8..aeaf95ddb77 100644 --- a/src/HtmlUtils.js +++ b/src/HtmlUtils.js @@ -468,7 +468,7 @@ export function bodyToHtml(content, highlights, opts={}) { // their username ( content.formatted_body == undefined || - !content.formatted_body.includes("https://matrix.to/") + !content.formatted_body.includes("https://matrix.to/") ); } @@ -513,3 +513,38 @@ export function linkifyElement(element, options = linkifyMatrix.options) { export function linkifyAndSanitizeHtml(dirtyHtml) { return sanitizeHtml(linkifyString(dirtyHtml), sanitizeHtmlParams); } + +/** + * Returns if a node is a block element or not. + * Only takes html nodes into account that are allowed in matrix messages. + * + * @param {Node} node + * @returns {bool} + */ +export function checkBlockNode(node) { + switch (node.nodeName) { + case "H1": + case "H2": + case "H3": + case "H4": + case "H5": + case "H6": + case "PRE": + case "BLOCKQUOTE": + case "DIV": + case "P": + case "UL": + case "OL": + case "LI": + case "HR": + case "TABLE": + case "THEAD": + case "TBODY": + case "TR": + case "TH": + case "TD": + return true; + default: + return false; + } +} diff --git a/src/components/views/messages/EditHistoryMessage.js b/src/components/views/messages/EditHistoryMessage.js index 8649cca1c49..7f00449156a 100644 --- a/src/components/views/messages/EditHistoryMessage.js +++ b/src/components/views/messages/EditHistoryMessage.js @@ -17,6 +17,7 @@ limitations under the License. import React from 'react'; import PropTypes from 'prop-types'; import * as HtmlUtils from '../../../HtmlUtils'; +import { editBodyDiffToHtml } from '../../../utils/MessageDiffUtils'; import {formatTime} from '../../../DateUtils'; import {MatrixEvent} from 'matrix-js-sdk'; import {pillifyLinks} from '../../../utils/pillify'; @@ -25,18 +26,12 @@ import sdk from '../../../index'; import MatrixClientPeg from '../../../MatrixClientPeg'; import Modal from '../../../Modal'; import classNames from 'classnames'; -import DiffMatchPatch from 'diff-match-patch'; function getReplacedContent(event) { const originalContent = event.getOriginalContent(); return originalContent["m.new_content"] || originalContent; } -function isPlainMessage(event) { - const content = getReplacedContent(event); - return content.msgtype === "m.text" && !content.format; -} - export default class EditHistoryMessage extends React.PureComponent { static propTypes = { // the message event being edited @@ -128,22 +123,6 @@ export default class EditHistoryMessage extends React.PureComponent { ); } - _renderBodyDiff(oldBody, newBody) { - const dpm = new DiffMatchPatch(); - const diff = dpm.diff_main(oldBody, newBody); - dpm.diff_cleanupSemantic(diff); - return diff.map(([modifier, text], i) => { - // not using del and ins tags here as del is used for strikethrough - if (modifier < 0) { - return ({text}); - } else if (modifier > 0) { - return ({text}); - } else { - return text; - } - }); - } - render() { const {mxEvent} = this.props; const content = getReplacedContent(mxEvent); @@ -153,8 +132,8 @@ export default class EditHistoryMessage extends React.PureComponent { contentContainer = ; } else { let contentElements; - if (isPlainMessage(mxEvent) && this.props.previousEdit && isPlainMessage(this.props.previousEdit)) { - contentElements = this._renderBodyDiff(getReplacedContent(this.props.previousEdit).body, content.body); + if (this.props.previousEdit) { + contentElements = editBodyDiffToHtml(getReplacedContent(this.props.previousEdit), content); } else { contentElements = HtmlUtils.bodyToHtml(content, null, {stripReplyFallback: true}); } diff --git a/src/editor/deserialize.js b/src/editor/deserialize.js index 6507a8dc122..8fefecb8857 100644 --- a/src/editor/deserialize.js +++ b/src/editor/deserialize.js @@ -17,6 +17,7 @@ limitations under the License. import { MATRIXTO_URL_PATTERN } from '../linkify-matrix'; import { walkDOMDepthFirst } from "./dom"; +import { checkBlockNode } from "../HtmlUtils"; const REGEX_MATRIXTO = new RegExp(MATRIXTO_URL_PATTERN); @@ -118,21 +119,6 @@ function checkDecendInto(node) { } } -function checkBlockNode(node) { - switch (node.nodeName) { - case "PRE": - case "BLOCKQUOTE": - case "DIV": - case "P": - case "UL": - case "OL": - case "LI": - return true; - default: - return false; - } -} - function checkIgnored(n) { if (n.nodeType === Node.TEXT_NODE) { // riot adds \n text nodes in a lot of places, diff --git a/src/utils/MessageDiffUtils.js b/src/utils/MessageDiffUtils.js new file mode 100644 index 00000000000..8c1035132b5 --- /dev/null +++ b/src/utils/MessageDiffUtils.js @@ -0,0 +1,258 @@ +/* +Copyright 2019 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import React from 'react'; +import classNames from 'classnames'; +import DiffMatchPatch from 'diff-match-patch'; +import {DiffDOM} from "diff-dom"; +import { checkBlockNode, bodyToHtml } from "../HtmlUtils"; + +const decodeEntities = (function() { + let textarea = null; + return function(string) { + if (!textarea) { + textarea = document.createElement("textarea"); + } + textarea.innerHTML = string; + return textarea.value; + }; +})(); + +function textToHtml(text) { + const container = document.createElement("div"); + container.textContent = text; + return container.innerHTML; +} + +function getSanitizedHtmlBody(content) { + const opts = { + stripReplyFallback: true, + returnString: true, + }; + if (content.format === "org.matrix.custom.html") { + return bodyToHtml(content, null, opts); + } else { + // convert the string to something that can be safely + // embedded in an html document, e.g. use html entities where needed + // This is also needed so that DiffDOM wouldn't interpret something + // as a tag when somebody types e.g. "" + + // as opposed to bodyToHtml, here we also render + // text messages with dangerouslySetInnerHTML, to unify + // the code paths and because we need html to show differences + return textToHtml(bodyToHtml(content, null, opts)); + } +} + +function wrapInsertion(child) { + const wrapper = document.createElement(checkBlockNode(child) ? "div" : "span"); + wrapper.className = "mx_EditHistoryMessage_insertion"; + wrapper.appendChild(child); + return wrapper; +} + +function wrapDeletion(child) { + const wrapper = document.createElement(checkBlockNode(child) ? "div" : "span"); + wrapper.className = "mx_EditHistoryMessage_deletion"; + wrapper.appendChild(child); + return wrapper; +} + +function findRefNodes(root, route, isAddition) { + let refNode = root; + let refParentNode; + const end = isAddition ? route.length - 1 : route.length; + for (let i = 0; i < end; ++i) { + refParentNode = refNode; + refNode = refNode.childNodes[route[i]]; + } + return {refNode, refParentNode}; +} + +function diffTreeToDOM(desc) { + if (desc.nodeName === "#text") { + return stringAsTextNode(desc.data); + } else { + const node = document.createElement(desc.nodeName); + if (desc.attributes) { + for (const [key, value] of Object.entries(desc.attributes)) { + node.setAttribute(key, value); + } + } + if (desc.childNodes) { + for (const childDesc of desc.childNodes) { + node.appendChild(diffTreeToDOM(childDesc)); + } + } + return node; + } +} + +function insertBefore(parent, nextSibling, child) { + if (nextSibling) { + parent.insertBefore(child, nextSibling); + } else { + parent.appendChild(child); + } +} + +function isRouteOfNextSibling(route1, route2) { + // routes are arrays with indices, + // to be interpreted as a path in the dom tree + + // ensure same parent + for (let i = 0; i < route1.length - 1; ++i) { + if (route1[i] !== route2[i]) { + return false; + } + } + // the route2 is only affected by the diff of route1 + // inserting an element if the index at the level of the + // last element of route1 being larger + // (e.g. coming behind route1 at that level) + const lastD1Idx = route1.length - 1; + return route2[lastD1Idx] >= route1[lastD1Idx]; +} + +function adjustRoutes(diff, remainingDiffs) { + if (diff.action === "removeTextElement" || diff.action === "removeElement") { + // as removed text is not removed from the html, but marked as deleted, + // we need to readjust indices that assume the current node has been removed. + const advance = 1; + for (const rd of remainingDiffs) { + if (isRouteOfNextSibling(diff.route, rd.route)) { + rd.route[diff.route.length - 1] += advance; + } + } + } +} + +function stringAsTextNode(string) { + return document.createTextNode(decodeEntities(string)); +} + +function renderDifferenceInDOM(originalRootNode, diff, diffMathPatch) { + const {refNode, refParentNode} = findRefNodes(originalRootNode, diff.route); + switch (diff.action) { + case "replaceElement": { + const container = document.createElement("span"); + const delNode = wrapDeletion(diffTreeToDOM(diff.oldValue)); + const insNode = wrapInsertion(diffTreeToDOM(diff.newValue)); + container.appendChild(delNode); + container.appendChild(insNode); + refNode.parentNode.replaceChild(container, refNode); + break; + } + case "removeTextElement": { + const delNode = wrapDeletion(stringAsTextNode(diff.value)); + refNode.parentNode.replaceChild(delNode, refNode); + break; + } + case "removeElement": { + const delNode = wrapDeletion(diffTreeToDOM(diff.element)); + refNode.parentNode.replaceChild(delNode, refNode); + break; + } + case "modifyTextElement": { + const textDiffs = diffMathPatch.diff_main(diff.oldValue, diff.newValue); + diffMathPatch.diff_cleanupSemantic(textDiffs); + const container = document.createElement("span"); + for (const [modifier, text] of textDiffs) { + let textDiffNode = stringAsTextNode(text); + if (modifier < 0) { + textDiffNode = wrapDeletion(textDiffNode); + } else if (modifier > 0) { + textDiffNode = wrapInsertion(textDiffNode); + } + container.appendChild(textDiffNode); + } + refNode.parentNode.replaceChild(container, refNode); + break; + } + case "addElement": { + const insNode = wrapInsertion(diffTreeToDOM(diff.element)); + insertBefore(refParentNode, refNode, insNode); + break; + } + case "addTextElement": { + if (diff.value !== "\n") { + const insNode = wrapInsertion(stringAsTextNode(diff.value)); + insertBefore(refParentNode, refNode, insNode); + } + break; + } + // e.g. when changing a the href of a link, + // show the link with old href as removed and with the new href as added + case "removeAttribute": + case "addAttribute": + case "modifyAttribute": { + const delNode = wrapDeletion(refNode.cloneNode(true)); + const updatedNode = refNode.cloneNode(true); + if (diff.action === "addAttribute" || diff.action === "modifyAttribute") { + updatedNode.setAttribute(diff.name, diff.newValue); + } else { + updatedNode.removeAttribute(diff.name); + } + const insNode = wrapInsertion(updatedNode); + const container = document.createElement(checkBlockNode(refNode) ? "div" : "span"); + container.appendChild(delNode); + container.appendChild(insNode); + refNode.parentNode.replaceChild(container, refNode); + break; + } + default: + // Should not happen (modifyComment, ???) + console.warn("MessageDiffUtils::editBodyDiffToHtml: diff action not supported atm", diff); + } +} + +/** + * Renders a message with the changes made in an edit shown visually. + * @param {object} originalContent the content for the base message + * @param {object} editContent the content for the edit message + * @return {object} a react element similar to what `bodyToHtml` returns + */ +export function editBodyDiffToHtml(originalContent, editContent) { + // wrap the body in a div, DiffDOM needs a root element + const originalBody = `
${getSanitizedHtmlBody(originalContent)}
`; + const editBody = `
${getSanitizedHtmlBody(editContent)}
`; + const dd = new DiffDOM(); + // diffActions is an array of objects with at least a `action` and `route` + // property. `action` tells us what the diff object changes, and `route` where. + // `route` is a path on the DOM tree expressed as an array of indices. + const diffActions = dd.diff(originalBody, editBody); + // for diffing text fragments + const diffMathPatch = new DiffMatchPatch(); + // parse the base html message as a DOM tree, to which we'll apply the differences found. + // fish out the div in which we wrapped the messages above with children[0]. + const originalRootNode = new DOMParser().parseFromString(originalBody, "text/html").body.children[0]; + for (let i = 0; i < diffActions.length; ++i) { + const diff = diffActions[i]; + renderDifferenceInDOM(originalRootNode, diff, diffMathPatch); + // DiffDOM assumes in subsequent diffs route path that + // the action was applied (e.g. that a removeElement action removed the element). + // This is not the case for us. We render differences in the DOM tree, and don't apply them. + // So we need to adjust the routes of the remaining diffs to account for this. + adjustRoutes(diff, diffActions.slice(i + 1)); + } + // take the html out of the modified DOM tree again + const safeBody = originalRootNode.innerHTML; + const className = classNames({ + 'mx_EventTile_body': true, + 'markdown-body': true, + }); + return ; +} diff --git a/yarn.lock b/yarn.lock index 8f6f24d5e7e..92312c28109 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2045,7 +2045,7 @@ commander@2.15.1: resolved "https://registry.yarnpkg.com/commander/-/commander-2.15.1.tgz#df46e867d0fc2aec66a34662b406a9ccafff5b0f" integrity sha512-VlfT9F3V0v+jr4yxPc5gg9s62/fIVWsd2Bk2iD435um1NlGMYdVCq+MjcXnhYq2icNOizHr1kK+5TI6H0Hy0ag== -commander@^2.11.0, commander@^2.20.0: +commander@^2.11.0, commander@^2.19.0, commander@^2.20.0: version "2.20.0" resolved "https://registry.yarnpkg.com/commander/-/commander-2.20.0.tgz#d58bb2b5c1ee8f87b0d340027e9e94e222c5a422" integrity sha512-7j2y+40w61zy6YC2iRNpUe/NwhNyoXrYpHMrSunaMG64nRnaf96zO/KMQR4OyN/UnE5KLyEBnKHd4aG3rskjpQ== @@ -2464,6 +2464,13 @@ di@^0.0.1: resolved "https://registry.yarnpkg.com/di/-/di-0.0.1.tgz#806649326ceaa7caa3306d75d985ea2748ba913c" integrity sha1-gGZJMmzqp8qjMG112YXqJ0i6kTw= +diff-dom@^4.1.3: + version "4.1.3" + resolved "https://registry.yarnpkg.com/diff-dom/-/diff-dom-4.1.3.tgz#5572dab5c549c7da05a7662303b471d7ea4b0963" + integrity sha512-wiM06O2BN3THf9Ge86IYuTek+OlKLSwwuwjm3XfSr7SGp7Ls9XXztpr4tp9S54RhX/rXqq94LdeBG+MMpDPDig== + dependencies: + rollup-plugin-terser "^4.0.3" + diff-match-patch@^1.0.4: version "1.0.4" resolved "https://registry.yarnpkg.com/diff-match-patch/-/diff-match-patch-1.0.4.tgz#6ac4b55237463761c4daf0dc603eb869124744b1" @@ -4453,6 +4460,14 @@ jest-regex-util@^24.3.0: resolved "https://registry.yarnpkg.com/jest-regex-util/-/jest-regex-util-24.3.0.tgz#d5a65f60be1ae3e310d5214a0307581995227b36" integrity sha512-tXQR1NEOyGlfylyEjg1ImtScwMq8Oh3iJbGTjN7p0J23EuVX1MA8rwU69K4sLbCmwzgCUbVkm0FkSF9TdzOhtg== +jest-worker@^24.0.0: + version "24.6.0" + resolved "https://registry.yarnpkg.com/jest-worker/-/jest-worker-24.6.0.tgz#7f81ceae34b7cde0c9827a6980c35b7cdc0161b3" + integrity sha512-jDwgW5W9qGNvpI1tNnvajh0a5IE/PuGLFmHk6aR/BZFz8tSgGw17GsDPXAJ6p91IvYDjOw8GpFbvvZGAK+DPQQ== + dependencies: + merge-stream "^1.0.1" + supports-color "^6.1.0" + jquery@^3.3.1: version "3.4.1" resolved "https://registry.yarnpkg.com/jquery/-/jquery-3.4.1.tgz#714f1f8d9dde4bdfa55764ba37ef214630d80ef2" @@ -5043,6 +5058,13 @@ meow@^5.0.0: trim-newlines "^2.0.0" yargs-parser "^10.0.0" +merge-stream@^1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/merge-stream/-/merge-stream-1.0.1.tgz#4041202d508a342ba00174008df0c251b8c135e1" + integrity sha1-QEEgLVCKNCugAXQAjfDCUbjBNeE= + dependencies: + readable-stream "^2.0.1" + merge2@^1.2.3: version "1.2.3" resolved "https://registry.yarnpkg.com/merge2/-/merge2-1.2.3.tgz#7ee99dbd69bb6481689253f018488a1b902b0ed5" @@ -6709,6 +6731,16 @@ ripemd160@^2.0.0, ripemd160@^2.0.1: hash-base "^3.0.0" inherits "^2.0.1" +rollup-plugin-terser@^4.0.3: + version "4.0.4" + resolved "https://registry.yarnpkg.com/rollup-plugin-terser/-/rollup-plugin-terser-4.0.4.tgz#6f661ef284fa7c27963d242601691dc3d23f994e" + integrity sha512-wPANT5XKVJJ8RDUN0+wIr7UPd0lIXBo4UdJ59VmlPCtlFsE20AM+14pe+tk7YunCsWEiuzkDBY3QIkSCjtrPXg== + dependencies: + "@babel/code-frame" "^7.0.0" + jest-worker "^24.0.0" + serialize-javascript "^1.6.1" + terser "^3.14.1" + run-async@^2.2.0: version "2.3.0" resolved "https://registry.yarnpkg.com/run-async/-/run-async-2.3.0.tgz#0371ab4ae0bdd720d4166d7dfda64ff7a445a6c0" @@ -6805,7 +6837,7 @@ selection-is-backward@^1.0.0: resolved "https://registry.yarnpkg.com/semver/-/semver-5.7.0.tgz#790a7cf6fea5459bac96110b29b60412dc8ff96b" integrity sha512-Ya52jSX2u7QKghxeoFGpLwCtGlt7j0oY9DYb5apt9nPlJ42ID+ulTXESnt/qAQcoSERyZ5sl3LDIOw0nAn/5DA== -serialize-javascript@^1.7.0: +serialize-javascript@^1.6.1, serialize-javascript@^1.7.0: version "1.7.0" resolved "https://registry.yarnpkg.com/serialize-javascript/-/serialize-javascript-1.7.0.tgz#d6e0dfb2a3832a8c94468e6eb1db97e55a192a65" integrity sha512-ke8UG8ulpFOxO8f8gRYabHQe/ZntKlcig2Mp+8+URDP1D8vJZ0KUt7LYo07q25Z/+JVSgpr/cui9PIp5H6/+nA== @@ -7093,7 +7125,7 @@ source-map-support@^0.4.15: dependencies: source-map "^0.5.6" -source-map-support@~0.5.12: +source-map-support@~0.5.10, source-map-support@~0.5.12: version "0.5.12" resolved "https://registry.yarnpkg.com/source-map-support/-/source-map-support-0.5.12.tgz#b4f3b10d51857a5af0138d3ce8003b201613d599" integrity sha512-4h2Pbvyy15EE02G+JOZpUCmqWJuqrs+sEkzewTm++BPi7Hvn/HwcqLAcNxYAyI0x13CpPPn+kMjl+hplXMHITQ== @@ -7546,6 +7578,15 @@ terser-webpack-plugin@^1.1.0: webpack-sources "^1.3.0" worker-farm "^1.7.0" +terser@^3.14.1: + version "3.17.0" + resolved "https://registry.yarnpkg.com/terser/-/terser-3.17.0.tgz#f88ffbeda0deb5637f9d24b0da66f4e15ab10cb2" + integrity sha512-/FQzzPJmCpjAH9Xvk2paiWrFq+5M6aVOf+2KRbwhByISDX/EujxsK+BAvrhb6H+2rtrLCHK9N01wO014vrIwVQ== + dependencies: + commander "^2.19.0" + source-map "~0.6.1" + source-map-support "~0.5.10" + terser@^4.0.0: version "4.1.2" resolved "https://registry.yarnpkg.com/terser/-/terser-4.1.2.tgz#b2656c8a506f7ce805a3f300a2ff48db022fa391"