Merge pull request #3244 from matrix-org/bwindels/diffhtmledits

Show diff for formatted messages in the edit history

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",

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);
     }

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;
+    }
+}

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 (<span className="mx_EditHistoryMessage_deletion" key={i}>{text}</span>);
-            } else if (modifier > 0) {
-                return (<span className="mx_EditHistoryMessage_insertion" key={i}>{text}</span>);
-            } else {
-                return text;
-            }
-        });
-    }
-
     render() {
         const {mxEvent} = this.props;
         const content = getReplacedContent(mxEvent);
@@ -153,8 +132,8 @@ export default class EditHistoryMessage extends React.PureComponent {
             contentContainer = <UnknownBody mxEvent={this.props.mxEvent} />;
         } 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});
             }

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,

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. "</sarcasm>"
+
+        // 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 = `<div>${getSanitizedHtmlBody(originalContent)}</div>`;
+    const editBody = `<div>${getSanitizedHtmlBody(editContent)}</div>`;
+    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 <span key="body" className={className} dangerouslySetInnerHTML={{ __html: safeBody }} dir="auto" />;
+}

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"