Close markers properly in editor-dom renderer

Switch to a more naive rendering strategy:
  * find the number of markups this marker has in common with its prev/next
    marker, slice off that number from its own markups and return that
    as the opened/closed markups.
  * This allows us to simply walk up the number of closed markups again.
  * The drawback is that this will cause some unnecessary open/closed markups
    (closing one only to immediately reopen it in the following marker, e.g.)

src/js/models/marker.js

@@ -5,7 +5,7 @@ import {
 } from '../utils/dom-utils';
 import {
   detect,
-  difference
+  commonItemLength
 } from 'content-kit-editor/utils/array-utils';
 import LinkedItem from "content-kit-editor/utils/linked-item";
 
@@ -103,11 +103,21 @@ const Marker = class Marker extends LinkedItem {
   }
 
   get openedMarkups() {
-    return difference(this.markups, (this.prev ? this.prev.markups : []));
+    let count = 0;
+    if (this.prev) {
+      count = commonItemLength(this.markups, this.prev.markups);
+    }
+
+    return this.markups.slice(count);
   }
 
   get closedMarkups() {
-    return difference(this.markups, (this.next ? this.next.markups : []));
+    let count = 0;
+    if (this.next) {
+      count = commonItemLength(this.markups, this.next.markups);
+    }
+
+    return this.markups.slice(count);
   }
 
 };

src/js/renderers/editor-dom.js

@@ -47,7 +47,6 @@ function getNextMarkerElement(renderNode) {
   let element = renderNode.element.parentNode;
   let closedCount = renderNode.postNode.closedMarkups.length;
 
-  // walk up the number of closed markups
   while (closedCount--) {
     element = element.parentNode;
   }
@@ -74,11 +73,9 @@ function renderMarker(marker, element, previousRenderNode) {
   }
 
   if (previousRenderNode) {
-    let nextMarkerElement = getNextMarkerElement(previousRenderNode);
-
     let previousSibling = previousRenderNode.element;
-    let previousSiblingPenultimate = penultimateParentOf(previousSibling, nextMarkerElement);
-    nextMarkerElement.insertBefore(currentElement, previousSiblingPenultimate.next);
+    let previousSiblingPenultimate = penultimateParentOf(previousSibling, element);
+    element.insertBefore(currentElement, previousSiblingPenultimate.nextSibling);
   } else {
     element.insertBefore(currentElement, element.firstChild);
   }

src/js/utils/array-utils.js

@@ -34,21 +34,6 @@ function forEach(enumerable, callback) {
   }
 }
 
-/**
- * @return {Array} The things in enumerable that are not in otherEnumerable,
- * aka the relative complement of `otherEnumerable` in `enumerable`
- */
-function difference(enumerable, otherEnumerable) {
-  const diff = [];
-  forEach(enumerable, (item) => {
-    if (otherEnumerable.indexOf(item) === -1) {
-      diff.push(item);
-    }
-  });
-
-  return diff;
-}
-
 function filter(enumerable, conditionFn) {
   const filtered = [];
   forEach(enumerable, i => {
@@ -57,10 +42,24 @@ function filter(enumerable, conditionFn) {
   return filtered;
 }
 
+/**
+ * @return {Integer} the number of items that are the same, starting from the 0th index, in a and b
+ */
+function commonItemLength(listA, listB) {
+  let offset = 0;
+  while (offset < listA.length && offset < listB.length) {
+    if (listA[offset] !== listB[offset]) {
+      break;
+    }
+    offset++;
+  }
+  return offset;
+}
+
 export {
   detect,
   forEach,
   any,
-  difference,
-  filter
+  filter,
+  commonItemLength
 };

tests/unit/renderers/editor-dom-test.js

@@ -394,8 +394,8 @@ test('render when contiguous markers have out-of-order markups', (assert) => {
         i = builder.createMarkup('I');
 
   const markers = [
-    builder.createMarker('bi', [b,i]),
-    builder.createMarker('ib', [i,b]),
+    builder.createMarker('BI', [b,i]),
+    builder.createMarker('IB', [i,b]),
     builder.createMarker('plain', [])
   ];
   const m1 = markers[0];
@@ -409,15 +409,36 @@ test('render when contiguous markers have out-of-order markups', (assert) => {
   render(renderTree);
 
   assert.equal(node.element.innerHTML,
-               '<p><b><i>biib</i></b>plain</p>');
+               '<p><b><i>BI</i></b><i><b>IB</b></i>plain</p>');
 
   // remove 'b' from 1st marker, rerender
   m1.removeMarkup(b);
   m1.renderNode.markDirty();
   render(renderTree);
 
   assert.equal(node.element.innerHTML,
-               '<p><i>bi<b>ib</b></i>plain</p>');
+               '<p><i>BI<b>IB</b></i>plain</p>');
+});
+
+test('contiguous markers have overlapping markups', (assert) => {
+  const b = builder.createMarkup('b'),
+        i = builder.createMarkup('i');
+  const post = builder.createPost();
+  const markers = [
+    builder.createMarker('W', [i]),
+    builder.createMarker('XY', [i,b]),
+    builder.createMarker('Z', [b])
+  ];
+  const section = builder.createMarkupSection('P', markers);
+  post.sections.append(section);
+
+  const node = new RenderNode(post);
+  const renderTree = new RenderTree(node);
+  node.renderTree = renderTree;
+  render(renderTree);
+
+  assert.equal(node.element.innerHTML,
+               '<p><i>W<b>XY</b></i><b>Z</b></p>');
 });
 
 /*