[bugfix] Ensure cursor positioning on a blank markup section or list …

…item works

`Cursor#_findNodeForPosition` should return the cursor element ("br",
usually) for a blank section. This change also ensures that the built-in
text-expansions for list items and headers position the cursor properly.

src/js/editor/text-input-handlers.js

@@ -32,6 +32,7 @@ function replaceWithHeaderSection(editor, headingTagName) {
     let { builder } = postEditor;
     let newSection = builder.createMarkupSection(headingTagName);
     postEditor.replaceSection(section, newSection);
+    postEditor.setRange(new Range(newSection.headPosition()));
   });
 }
 

src/js/models/render-node.js

@@ -12,6 +12,7 @@ export default class RenderNode extends LinkedItem {
     this.postNode = postNode;
     this._childNodes = null;
     this._element = null;
+    this._cursorElement = null; // blank render nodes need a cursor element
     this.renderTree = renderTree;
 
     // RenderNodes for Markers keep track of their markupElement
@@ -68,6 +69,12 @@ export default class RenderNode extends LinkedItem {
   get element() {
     return this._element;
   }
+  set cursorElement(cursorElement) {
+    this._cursorElement = cursorElement;
+  }
+  get cursorElement() {
+    return this._cursorElement || this.element;
+  }
   destroy() {
     this.element = null;
     this.parent = null;

src/js/renderers/editor-dom.js

@@ -327,10 +327,13 @@ class Visitor {
     // Always rerender the section -- its tag name or attributes may have changed.
     // TODO make this smarter, only rerendering and replacing the element when necessary
     renderNode.element = renderMarkupSection(section);
+    renderNode.cursorElement = null;
     attachRenderNodeElementToDOM(renderNode, originalElement);
 
     if (section.isBlank) {
-      renderNode.element.appendChild(renderCursorPlaceholder());
+      let cursorPlaceholder = renderCursorPlaceholder();
+      renderNode.element.appendChild(cursorPlaceholder);
+      renderNode.cursorElement = cursorPlaceholder;
     } else {
       const visitAll = true;
       visit(renderNode, section.markers, visitAll);
@@ -350,10 +353,13 @@ class Visitor {
   [LIST_ITEM_TYPE](renderNode, item, visit) {
     // FIXME do we need to do anything special for rerenders?
     renderNode.element = renderListItem();
+    renderNode.cursorElement = null;
     attachRenderNodeElementToDOM(renderNode, null);
 
     if (item.isBlank) {
-      renderNode.element.appendChild(renderCursorPlaceholder());
+      let cursorPlaceholder = renderCursorPlaceholder();
+      renderNode.element.appendChild(cursorPlaceholder);
+      renderNode.cursorElement = cursorPlaceholder;
     } else {
       const visitAll = true;
       visit(renderNode, item.markers, visitAll);

src/js/utils/cursor.js

@@ -84,7 +84,7 @@ const Cursor = class Cursor {
         node = section.renderNode.element.lastChild;
       }
     } else if (section.isBlank) {
-      node = section.renderNode.element;
+      node = section.renderNode.cursorElement;
       offset = 0;
     } else {
       let {marker, offsetInMarker} = position;

tests/acceptance/editor-input-handlers-test.js

@@ -45,6 +45,23 @@ headerTests.forEach(({text, toInsert, headerTagName}) => {
     assert.hasNoElement('#editor p', 'p is gone');
     assert.hasElement(`#editor ${headerTagName}`, `p -> ${headerTagName}`);
 
+    // Different browsers report different selections, so we grab the selection
+    // here and then set it to what we expect it to be, and compare what
+    // window.getSelection() reports.
+    // E.g., in Firefox getSelection() reports that the anchorNode is the "br",
+    // but Safari and Chrome report that the anchorNode is the header element
+    let selection = window.getSelection();
+
+    let cursorElement = $(`#editor ${headerTagName} br`)[0];
+    assert.ok(cursorElement, 'has cursorElement');
+    Helpers.dom.selectRange(cursorElement, 0, cursorElement, 0);
+
+    let newSelection = window.getSelection();
+    assert.equal(selection.anchorNode, newSelection.anchorNode, 'correct anchorNode');
+    assert.equal(selection.focusNode, newSelection.focusNode, 'correct focusNode');
+    assert.equal(selection.anchorOffset, newSelection.anchorOffset, 'correct anchorOffset');
+    assert.equal(selection.focusOffset, newSelection.focusOffset, 'correct focusOffset');
+
     Helpers.dom.insertText(editor, 'X');
     assert.hasElement(`#editor ${headerTagName}:contains(X)`, 'text is inserted correctly');
   });
@@ -70,12 +87,17 @@ test('typing "* " converts to ul > li', (assert) => {
   assert.hasNoElement('#editor p', 'p is gone');
   assert.hasElement('#editor ul > li', 'p -> "ul > li"');
 
-  let li = $('#editor ul > li')[0];
-  assert.ok(li, 'has li for cursor position');
-
+  // Store the selection so we can compare later
   let selection = window.getSelection();
-  assert.equal(selection.anchorNode, li, 'selection anchorNode is li');
-  assert.equal(selection.focusNode, li, 'selection focusNode is li');
+  let cursorElement = $('#editor ul > li > br')[0];
+  assert.ok(cursorElement, 'has cursorElement for cursor position');
+  Helpers.dom.selectRange(cursorElement, 0, cursorElement, 0);
+
+  let newSelection = window.getSelection();
+  assert.equal(selection.anchorNode, newSelection.anchorNode, 'correct anchorNode');
+  assert.equal(selection.focusNode, newSelection.focusNode, 'correct focusNode');
+  assert.equal(selection.anchorOffset, newSelection.anchorOffset, 'correct anchorOffset');
+  assert.equal(selection.focusOffset, newSelection.focusOffset, 'correct focusOffset');
 
   Helpers.dom.insertText(editor, 'X');
   assert.hasElement('#editor ul > li:contains(X)', 'text is inserted correctly');
@@ -118,6 +140,19 @@ test('typing "1 " converts to ol > li', (assert) => {
   Helpers.dom.insertText(editor, ' ');
   assert.hasNoElement('#editor p', 'p is gone');
   assert.hasElement('#editor ol > li', 'p -> "ol > li"');
+
+  // Store the selection so we can compare later
+  let selection = window.getSelection();
+  let cursorElement = $('#editor ol > li > br')[0];
+  assert.ok(cursorElement, 'has cursorElement for cursor position');
+  Helpers.dom.selectRange(cursorElement, 0, cursorElement, 0);
+
+  let newSelection = window.getSelection();
+  assert.equal(selection.anchorNode, newSelection.anchorNode, 'correct anchorNode');
+  assert.equal(selection.focusNode, newSelection.focusNode, 'correct focusNode');
+  assert.equal(selection.anchorOffset, newSelection.anchorOffset, 'correct anchorOffset');
+  assert.equal(selection.focusOffset, newSelection.focusOffset, 'correct focusOffset');
+
   Helpers.dom.insertText(editor, 'X');
 
   assert.hasElement('#editor li:contains(X)', 'text is inserted correctly');

tests/acceptance/editor-selections-test.js

@@ -308,9 +308,18 @@ test('selecting all text across sections and hitting enter deletes and moves cur
   assert.equal($('#editor p').length, 1, 'single section');
   assert.equal($('#editor p:eq(0)').text(), '', 'blank text');
 
-  assert.deepEqual(Helpers.dom.getCursorPosition(),
-                  {node: $('#editor p')[0], offset: 0},
-                  'cursor is at start of second section');
+  // Firefox reports that the cursor is on the "<br>", but Safari and Chrome do not.
+  // Grab the selection here, then set it to the expected value, and compare again
+  // the window's selection
+  let selection = window.getSelection();
+  let cursorElement = $('#editor p br')[0];
+  assert.ok(cursorElement, 'has cursor element');
+  Helpers.dom.selectRange(cursorElement, 0, cursorElement, 0);
+  let newSelection = window.getSelection();
+  assert.equal(selection.anchorNode, newSelection.anchorNode, 'correct anchorNode');
+  assert.equal(selection.focusNode, newSelection.focusNode, 'correct focusNode');
+  assert.equal(selection.anchorOffset, newSelection.anchorOffset, 'correct anchorOffset');
+  assert.equal(selection.focusOffset, newSelection.focusOffset, 'correct focusOffset');
 });
 
 test('selecting text across markup and list sections', (assert) => {