mark original event as replaced instead of replacing the event object

this is more in line with what happens on the server-side,
and also doesn't break existing reply relations.

src/models/event-timeline-set.js

@@ -243,31 +243,6 @@ EventTimelineSet.prototype.findEventById = function(eventId) {
     });
 };
 
-/**
- * Find an event by id which is stored in our timelines and replace it with a new event
- *
- * @param {string} eventId  event ID to look for
- * @param {string} newEvent the event to replace with
- * @return {?module:models/event~MatrixEvent} the replaced event, or undefined if unknown
- */
-EventTimelineSet.prototype.tryReplaceEvent = function(eventId, newEvent) {
-    // we can't use this._eventIdToTimeline here to find the matching timeline
-    // because we need to match the original event id,
-    // which might have been replaced by a previous m.replace already.
-    // for now, loop through all timelines. Maybe a separate index would be justifyable.
-    let oldEvent;
-    for(const tl of this._timelines) {
-        oldEvent = tl.tryReplaceEvent(eventId, newEvent);
-        if (oldEvent) {
-            break;
-        }
-    }
-    if (oldEvent) {
-        this.replaceEventId(oldEvent.getId(), newEvent.getId());
-        return oldEvent;
-    }
-};
-
 /**
  * Add a new timeline to this timeline list
  *

src/models/event-timeline.js

@@ -339,27 +339,6 @@ EventTimeline.prototype.addEvent = function(event, atStart) {
     }
 };
 
-
-/**
- * Replaces an event in the timeline, if it exists.
- *
- * @param {string} eventId  the id of the existing event
- * @param {boolean} newEvent  the new event
- * @return {MatrixEvent?} the old event, if found and replaced.
- */
-EventTimeline.prototype.tryReplaceEvent = function(eventId, newEvent) {
-    const eventIndex = this._events.findIndex(event => event.getOriginalId() === eventId);
-    if (eventIndex === -1) {
-        return;
-    }
-    const oldEvent = this._events[eventIndex];
-    this._events[eventIndex] = newEvent;
-    // copy over metadata, as we don't hold state for random locations in the timeline, only for the end or start
-    // and assuming for now that replacing an event doesn't change the sender that should be shown.
-    newEvent.sender = oldEvent.sender;
-    return oldEvent;
-};
-
 /**
  * Static helper method to set sender and target properties
  *

src/models/event.js

@@ -122,7 +122,7 @@ module.exports.MatrixEvent = function MatrixEvent(
     this.error = null;
     this.forwardLooking = true;
     this._pushActions = null;
-    this._replacedEvent = null;
+    this._replacingEvent = null;
 
     this._clearEvent = {};
 
@@ -209,9 +209,6 @@ utils.extend(module.exports.MatrixEvent.prototype, {
      * @return {Number} The event timestamp, e.g. <code>1433502692297</code>
      */
     getTs: function() {
-        if (this._replacedEvent) {
-            return this._replacedEvent.getTs();
-        }
         return this.event.origin_server_ts;
     },
 
@@ -220,9 +217,6 @@ utils.extend(module.exports.MatrixEvent.prototype, {
      * @return {Date} The event date, e.g. <code>new Date(1433502692297)</code>
      */
     getDate: function() {
-        if (this._replacedEvent) {
-            return this._replacedEvent.getDate();
-        }
         return this.event.origin_server_ts ? new Date(this.event.origin_server_ts) : null;
     },
 
@@ -232,7 +226,13 @@ utils.extend(module.exports.MatrixEvent.prototype, {
      * @return {Object} The event content JSON, or an empty object.
      */
     getContent: function() {
-        return this._clearEvent.content || this.event.content || {};
+        const content = this._clearEvent.content || this.event.content || {};
+        if (this._replacingEvent) {
+            const newContent = this._replacingEvent.getContent()["m.new_content"];
+            return Object.assign({}, newContent, content["m.relates_to"]);
+        } else {
+            return content;
+        }
     },
 
     /**
@@ -741,37 +741,34 @@ utils.extend(module.exports.MatrixEvent.prototype, {
     },
 
     /**
-     * Get whether the event is a relation event.
+     * Get whether the event is a relation event, and of a given type if `relType` is passed in.
+     *
+     * @param {string?} relType if given, checks that the relation is of the given type
      * @return {boolean}
      */
-    isRelation() {
+    isRelation(relType = undefined) {
         const content = this.getContent();
         const relation = content && content["m.relates_to"];
-        return relation && relation.rel_type && relation.event_id;
+        return relation && relation.rel_type && relation.event_id &&
+            ((relType && relation.rel_type === relType) || !relType);
     },
 
-    isReplacement() {
-        const content = this.getContent();
-        const relation = content && content["m.relates_to"];
-        return relation && relation.rel_type === "m.replace" && relation.event_id;
+    /**
+     * Set an event that replaces the content of this event, through an m.replace relation.
+     *
+     * @param {MatrixEvent} newEvent the event with the replacing content.
+     */
+    makeReplaced(newEvent) {
+        this._replacingEvent = newEvent;
     },
 
-    setReplacedEvent(replacedEvent) {
-        this._replacedEvent = replacedEvent;
-    },
-
-    getReplacedEvent() {
-        return this._replacedEvent;
-    },
-
-    getOriginalId() {
-        const content = this.getContent();
-        const relation = content && content["m.relates_to"];
-        if (relation && relation.rel_type === "m.replace") {
-            return relation.event_id;
-        } else {
-            return this.getId();
-        }
+    /**
+     * Returns the event ID of the event replacing the content of this event, if any.
+     *
+     * @return {string?}
+     */
+    replacingEventId() {
+        return this._replacingEvent && this._replacingEvent.getId();
     },
 });
 

src/models/room.js

@@ -1032,22 +1032,15 @@ Room.prototype._addLiveEvent = function(event, duplicateStrategy) {
         // this may be needed to trigger an update.
     }
 
-    if (this._opts.unstableClientRelationReplacements && event.isReplacement()) {
-        const replacedEventId = event.getOriginalId();
-        const replacedEvent = replacedEventId &&
-            this.getUnfilteredTimelineSet().tryReplaceEvent(replacedEventId, event);
+
+    if (this._opts.unstableClientRelationReplacements && event.isRelation("m.replace")) {
+        const relatesTo = event.getContent()["m.relates_to"];
+        const replacedId = relatesTo && relatesTo.event_id;
+        const replacedEvent = this.getUnfilteredTimelineSet().findEventById(replacedId);
         if (replacedEvent) {
-            // if this was already a replacement, get the original
-            let originalEvent = replacedEvent;
-            if (originalEvent.isReplacement()) {
-                originalEvent = originalEvent.getReplacedEvent();
+            replacedEvent.makeReplaced(event);
+            this.emit("Room.replaceEvent", replacedEvent, this);
         }
-            event.setReplacedEvent(originalEvent);
-            // report replacedEvent and not originalEvent because replaceEvent was in the timeline so far
-            this.emit("Room.replaceEvent", replacedEvent, event, this);
-        }
-        // we don't add the event because the event type would get rendered
-        return;
     }
 
     if (event.getUnsigned().transaction_id) {
@@ -1101,7 +1094,7 @@ Room.prototype._addLiveEvent = function(event, duplicateStrategy) {
  * unique transaction id.
  */
 Room.prototype.addPendingEvent = function(event, txnId) {
-    if (event.isReplacement()) {
+    if (event.isRelation("m.replace")) {
         return;
     }