From 7a9c631b9e9359fc621f8f9bbf0d0fc33bb2b622 Mon Sep 17 00:00:00 2001 From: Nate Mielnik Date: Fri, 7 Aug 2015 18:12:27 -0400 Subject: [PATCH] Fix issues with import/export selection within nested blocks Conflicts: src/js/selection.js src/js/util.js --- spec/selection.spec.js | 56 ++++++++++++++++++++++++++++++------------ src/js/core.js | 16 +----------- src/js/selection.js | 53 +++++++++++++++++++++++++++++++++++++-- src/js/util.js | 17 +++++++++++-- 4 files changed, 107 insertions(+), 35 deletions(-) diff --git a/spec/selection.spec.js b/spec/selection.spec.js index 3ef1d7aa2..b822324ae 100644 --- a/spec/selection.spec.js +++ b/spec/selection.spec.js @@ -1,5 +1,5 @@ /*global MediumEditor, describe, it, expect, spyOn, - afterEach, beforeEach, fireEvent, Util, + afterEach, beforeEach, fireEvent, jasmine, selectElementContents, setupTestHelpers, selectElementContentsAndFire, Selection, placeCursorInsideElement */ @@ -195,7 +195,7 @@ describe('Selection TestCase', function () { 'emptyBlocksIndex': 1 }); - var startParagraph = Util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'p'); + var startParagraph = MediumEditor.util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'p'); expect(startParagraph).toBe(editor.elements[0].getElementsByTagName('p')[1], 'empty paragraph'); }); @@ -210,7 +210,7 @@ describe('Selection TestCase', function () { 'emptyBlocksIndex': 2 }); - var startParagraph = Util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'p'); + var startParagraph = MediumEditor.util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'p'); expect(startParagraph).toBe(editor.elements[0].getElementsByTagName('p')[2], 'paragraph after empty paragraph'); }); @@ -226,7 +226,7 @@ describe('Selection TestCase', function () { 'emptyBlocksIndex': 2 }); - var startParagraph = Util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'p'); + var startParagraph = MediumEditor.util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'p'); expect(startParagraph).toBe(editor.elements[1].getElementsByTagName('p')[2], 'paragraph after empty paragraph'); }); @@ -241,7 +241,7 @@ describe('Selection TestCase', function () { 'emptyBlocksIndex': 2 }); - var startParagraph = Util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'h2'); + var startParagraph = MediumEditor.util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'h2'); expect(startParagraph).toBe(editor.elements[0].querySelector('h2'), 'block element after empty block element'); }); @@ -256,7 +256,7 @@ describe('Selection TestCase', function () { 'emptyBlocksIndex': 2 }); - var startParagraph = Util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'h2'); + var startParagraph = MediumEditor.util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'h2'); expect(startParagraph).toBe(editor.elements[0].querySelector('h2'), 'block element after empty block element'); }); @@ -272,7 +272,7 @@ describe('Selection TestCase', function () { }); var innerElement = window.getSelection().getRangeAt(0).startContainer; - expect(Util.isDescendant(editor.elements[0].querySelector('i'), innerElement, true)).toBe(true, 'nested inline elment inside block element after empty block element'); + expect(MediumEditor.util.isDescendant(editor.elements[0].querySelector('i'), innerElement, true)).toBe(true, 'nested inline elment inside block element after empty block element'); }); ['br', 'img'].forEach(function (tagName) { @@ -310,7 +310,7 @@ describe('Selection TestCase', function () { var innerElement = window.getSelection().getRangeAt(0).startContainer; // The behavior varies from browser to browser for this case, some select TH, some #textNode - expect(Util.isDescendant(editor.elements[0].querySelector('th'), innerElement, true)) + expect(MediumEditor.util.isDescendant(editor.elements[0].querySelector('th'), innerElement, true)) .toBe(true, 'expect selection to be of TH or a descendant'); expect(innerElement).toBe(window.getSelection().getRangeAt(0).endContainer); }); @@ -328,8 +328,32 @@ describe('Selection TestCase', function () { }); var innerElement = window.getSelection().getRangeAt(0).startContainer; - expect(Util.isDescendant(editor.elements[0].querySelectorAll('p')[1], innerElement, true)).toBe(false, 'moved selection beyond non-empty block element'); - expect(Util.isDescendant(editor.elements[0].querySelector('h2'), innerElement, true)).toBe(true, 'moved selection to element to incorrect block element'); + expect(MediumEditor.util.isDescendant(editor.elements[0].querySelectorAll('p')[1], innerElement, true)).toBe(false, 'moved selection beyond non-empty block element'); + expect(MediumEditor.util.isDescendant(editor.elements[0].querySelector('h2'), innerElement, true)).toBe(true, 'moved selection to element to incorrect block element'); + }); + + // https://github.com/yabwe/medium-editor/issues/732 + it('should support a selection correctly when space + newlines are separating block elements', function () { + this.el.innerHTML = ''; + var editor = this.newMediumEditor('.editor'), + lastLi = this.el.querySelectorAll('ul > li')[2]; + + // Select the
  • with 'target' + selectElementContents(lastLi.firstChild); + + var selectionData = editor.exportSelection(); + expect(selectionData.emptyBlocksIndex).toBe(0); + + editor.importSelection(selectionData); + var range = window.getSelection().getRangeAt(0); + + expect(range.toString()).toBe('target', 'The selection is around the wrong element'); + expect(MediumEditor.util.isDescendant(lastLi, range.startContainer, true)).toBe(true, 'The start of the selection is invalid'); + expect(MediumEditor.util.isDescendant(lastLi, range.endContainer, true)).toBe(true, 'The end of the selection is invalid'); }); }); @@ -439,7 +463,7 @@ describe('Selection TestCase', function () { describe('getSelectedElements', function () { it('no selected elements on empty selection', function () { - var elements = Selection.getSelectedElements(document); + var elements = MediumEditor.selection.getSelectedElements(document); expect(elements.length).toBe(0); }); @@ -450,7 +474,7 @@ describe('Selection TestCase', function () { elements; selectElementContents(editor.elements[0].querySelector('i').firstChild); - elements = Selection.getSelectedElements(document); + elements = MediumEditor.selection.getSelectedElements(document); expect(elements.length).toBe(1); expect(elements[0].tagName.toLowerCase()).toBe('i'); @@ -462,7 +486,7 @@ describe('Selection TestCase', function () { var elements; selectElementContents(this.el); - elements = Selection.getSelectedElements(document); + elements = MediumEditor.selection.getSelectedElements(document); expect(elements.length).toBe(1); expect(elements[0].tagName.toLowerCase()).toBe('i'); @@ -472,8 +496,8 @@ describe('Selection TestCase', function () { describe('getSelectedParentElement', function () { it('should return null on bad range', function () { - expect(Selection.getSelectedParentElement(null)).toBe(null); - expect(Selection.getSelectedParentElement(false)).toBe(null); + expect(MediumEditor.selection.getSelectedParentElement(null)).toBe(null); + expect(MediumEditor.selection.getSelectedParentElement(false)).toBe(null); }); it('should select the document', function () { @@ -488,7 +512,7 @@ describe('Selection TestCase', function () { sel.removeAllRanges(); sel.addRange(range); - element = Selection.getSelectedParentElement(range); + element = MediumEditor.selection.getSelectedParentElement(range); expect(element).toBe(document); }); diff --git a/src/js/core.js b/src/js/core.js index b6368ef55..5a9eb9f1c 100644 --- a/src/js/core.js +++ b/src/js/core.js @@ -1003,21 +1003,7 @@ function MediumEditor(elements, options) { } if (typeof inSelectionState.emptyBlocksIndex !== 'undefined') { - var targetNode = Util.getBlockContainer(range.startContainer), - index = 0; - // Skip over empty blocks until we hit the block we want the selection to be in - while ((index === 0 || index < inSelectionState.emptyBlocksIndex) && targetNode.nextSibling) { - targetNode = targetNode.nextSibling; - index++; - // If we find a non-empty block, ignore the emptyBlocksIndex and just put selection here - if (targetNode.textContent.length > 0) { - break; - } - } - - // We're selecting a high-level block node, so make sure the cursor gets moved into the deepest - // element at the beginning of the block - range.setStart(Util.getFirstSelectableLeafNode(targetNode), 0); + range = Selection.importSelectionMoveCursorPastBlocks(this.options.ownerDocument, editableElement, inSelectionState.emptyBlocksIndex, range); } // If the selection is right at the ending edge of a link, put it outside the anchor tag instead of inside. diff --git a/src/js/selection.js b/src/js/selection.js index cd796d02f..1d8b4f508 100644 --- a/src/js/selection.js +++ b/src/js/selection.js @@ -67,6 +67,54 @@ var Selection; return range; }, + // Uses the emptyBlocksIndex calculated by getIndexRelativeToAdjacentEmptyBlocks + // to move the cursor back to the start of the correct paragraph + importSelectionMoveCursorPastBlocks: function (doc, root, index, range) { + var treeWalker = doc.createTreeWalker(root, NodeFilter.SHOW_ELEMENT, filterOnlyParentElements, false), + startContainer = range.startContainer, + startBlock, + targetNode, + currIndex = 0; + index = index || 1; // If index is 0, we still want to move to the next block + + // Chrome counts newlines and spaces that separate block elements as actual elements. + // If the selection is inside one of these text nodes, and it has a previous sibling + // which is a block element, we want the treewalker to start at the previous sibling + // and NOT at the parent of the textnode + if (startContainer.nodeType === 3 && Util.isBlockContainer(startContainer.previousSibling)) { + startBlock = startContainer.previousSibling; + } else { + startBlock = Util.getClosestBlockContainer(startContainer); + } + + // Skip over empty blocks until we hit the block we want the selection to be in + while (treeWalker.nextNode()) { + if (!targetNode) { + // Loop through all blocks until we hit the starting block element + if (startBlock === treeWalker.currentNode) { + targetNode = treeWalker.currentNode; + } + } else { + targetNode = treeWalker.currentNode; + currIndex++; + // We hit the target index, bail + if (currIndex === index) { + break; + } + // If we find a non-empty block, ignore the emptyBlocksIndex and just put selection here + if (targetNode.textContent.length > 0) { + break; + } + } + } + + // We're selecting a high-level block node, so make sure the cursor gets moved into the deepest + // element at the beginning of the block + range.setStart(Util.getFirstSelectableLeafNode(targetNode), 0); + + return range; + }, + // Returns -1 unless the cursor is at the beginning of a paragraph/block // If the paragraph/block is preceeded by empty paragraphs/block (with no text) // it will return the number of empty paragraphs before the cursor. @@ -89,14 +137,15 @@ var Selection; // Walk over block elements, counting number of empty blocks between last piece of text // and the block the cursor is in - var treeWalker = doc.createTreeWalker(root, NodeFilter.SHOW_ELEMENT, filterOnlyParentElements, false), + var closestBlock = Util.getClosestBlockContainer(cursorContainer), + treeWalker = doc.createTreeWalker(root, NodeFilter.SHOW_ELEMENT, filterOnlyParentElements, false), emptyBlocksCount = 0; while (treeWalker.nextNode()) { var blockIsEmpty = treeWalker.currentNode.textContent === ''; if (blockIsEmpty || emptyBlocksCount > 0) { emptyBlocksCount += 1; } - if (Util.isDescendant(treeWalker.currentNode, cursorContainer, true)) { + if (treeWalker.currentNode === closestBlock) { return emptyBlocksCount; } if (!blockIsEmpty) { diff --git a/src/js/util.js b/src/js/util.js index ab5f7236a..c3affac60 100644 --- a/src/js/util.js +++ b/src/js/util.js @@ -106,7 +106,14 @@ var Util; return true; }, - parentElements: ['p', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'blockquote', 'pre'], + parentElements: [ + // elements our editor generates + 'p', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'blockquote', 'pre', 'ul', 'li', 'ol', + // all other known block elements + 'address', 'article', 'aside', 'audio', 'canvas', 'dd', 'dl', 'dt', 'fieldset', + 'figcaption', 'figure', 'footer', 'form', 'header', 'hgroup', 'main', 'nav', + 'noscript', 'output', 'section', 'table', 'tbody', 'tfoot', 'video' + ], extend: function extend(/* dest, source1, source2, ...*/) { var args = [true].concat(Array.prototype.slice.call(arguments)); @@ -396,7 +403,7 @@ var Util; var parentNode = node.parentNode, tagName = parentNode.tagName.toLowerCase(); - while (!this.isBlockContainer(parentNode) && tagName !== 'div') { + while (tagName === 'li' || (!this.isBlockContainer(parentNode) && tagName !== 'div')) { if (tagName === 'li') { return true; } @@ -673,6 +680,12 @@ var Util; return element && element.nodeType !== 3 && this.parentElements.indexOf(element.nodeName.toLowerCase()) !== -1; }, + getClosestBlockContainer: function (node) { + return Util.traverseUp(node, function (node) { + return Util.isBlockContainer(node); + }); + }, + getBlockContainer: function (element) { return this.traverseUp(element, function (el) { return Util.isBlockContainer(el) && !Util.isBlockContainer(el.parentNode);