Fix bugs in deleteAtRange (#1072)

* Fix bug in deleteAtRange when selecting part of non-void block followed by void block

* Fix bug in deleteAtRange when selecting from an inline void node

* Fix bug with deleteAtRange if a whole startBlock was selected, fixes #1021

src/changes/at-range.js

@@ -135,6 +135,16 @@ Changes.deleteAtRange = (change, range, options = {}) => {
     }
   }
 
+  // If the selection starts at an inline void, remove that void inline first
+  const startInline = document.getClosestInline(startKey)
+  if (startInline && startInline.isVoid &&
+      startInline.getTexts().first().key == startKey) {
+    const nextText = document.getNextText(startInline.getTexts().first().key)
+    change.removeNodeByKey(startInline.key, OPTS)
+    startKey = nextText.key
+    startOffset = 0
+  }
+
   // If the start and end key are the same, we can just remove it.
   if (startKey == endKey) {
     // If it is a void node, remove the whole node
@@ -178,10 +188,12 @@ Changes.deleteAtRange = (change, range, options = {}) => {
   ancestor = document.getCommonAncestor(startKey, endKey)
   startChild = ancestor.getFurthestAncestor(startKey)
   endChild = ancestor.getFurthestAncestor(endKey)
+
+
+  const nextText = document.getNextText(endKey)
   const startIndex = ancestor.nodes.indexOf(startChild)
   const endIndex = ancestor.nodes.indexOf(endChild)
   const middles = ancestor.nodes.slice(startIndex + 1, endIndex + 1)
-  const next = document.getNextText(endKey)
 
   // Remove all of the middle nodes, between the splits.
   if (middles.size) {
@@ -190,16 +202,25 @@ Changes.deleteAtRange = (change, range, options = {}) => {
     })
   }
 
-  // If the start and end block are different, move all of the nodes from the
+  // Refresh variables
-  // end block into the start block.
   state = change.state
   document = state.document
-  const startBlock = document.getClosestBlock(startKey)
-  const endBlock = document.getClosestBlock(next.key)
 
-  // If the endBlock is void, just remove the startBlock
+  const startBlock = document.getClosestBlock(startKey)
-  if (endBlock.isVoid) {
+  const endBlock = document.getClosestBlock(nextText.key)
-    change.removeNodeByKey(startBlock.key)
+
+  // If the whole startBlock is selected but the endBlock is different, just remove the startBlock
+  if (startBlock.key !== endBlock.key && startChild.text.length === endOffset && startOffset === 0) {
+    document = change.removeNodeByKey(startBlock.key, OPTS).state.document
+    return
+  }
+
+  // If the endBlock is void, remove what is selected of the start block
+  if (endBlock.isVoid && endOffset === 0) {
+    // If part of the startBlock is selected, split it and remove the unwanted part
+    document = change.splitNodeByKey(startChild.key, startOffset, OPTS).state.document
+    const toBeRemoved = document.nodes.get(startIndex + 1)
+    change.removeNodeByKey(toBeRemoved.key, OPTS)
     return
   }
 

test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-up-to-start-of-void/index.js

@@ -0,0 +1,37 @@
+
+import assert from 'assert'
+
+export default function (state) {
+  const { document, selection } = state
+  const texts = document.getTexts()
+  const first = texts.first()
+  const second = texts.get(1)
+  const range = selection.merge({
+    anchorKey: first.key,
+    anchorOffset: 4,
+    focusKey: second.key,
+    focusOffset: 0
+  })
+
+  const next = state
+    .change()
+    .select(range)
+    .delete()
+    .state
+
+  const anchorAndFocusKey = next.document.getTexts().first()
+  assert.deepEqual(
+    next.selection.toJS(),
+    {
+      anchorKey: anchorAndFocusKey.key,
+      anchorOffset: anchorAndFocusKey.characters.size,
+      focusKey: anchorAndFocusKey.key,
+      focusOffset: anchorAndFocusKey.characters.size,
+      isBackward: false,
+      isFocused: false,
+      marks: null
+    }
+  )
+
+  return next
+}

test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-up-to-start-of-void/input.yaml

@@ -0,0 +1,15 @@
+
+nodes:
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: one two
+  - kind: block
+    type: image
+    isVoid: true
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: three

test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-up-to-start-of-void/output.yaml

@@ -0,0 +1,15 @@
+
+nodes:
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: "one "
+  - kind: block
+    type: image
+    isVoid: true
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: three

test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-void/index.js

@@ -0,0 +1,37 @@
+
+import assert from 'assert'
+
+export default function (state) {
+  const { document, selection } = state
+  const texts = document.getTexts()
+  const first = texts.first()
+  const second = texts.get(1)
+  const range = selection.merge({
+    anchorKey: first.key,
+    anchorOffset: 4,
+    focusKey: second.key,
+    focusOffset: 1
+  })
+
+  const next = state
+    .change()
+    .select(range)
+    .delete()
+    .state
+
+  const anchorAndFocusKey = next.document.getTexts().first()
+  assert.deepEqual(
+    next.selection.toJS(),
+    {
+      anchorKey: anchorAndFocusKey.key,
+      anchorOffset: anchorAndFocusKey.characters.size,
+      focusKey: anchorAndFocusKey.key,
+      focusOffset: anchorAndFocusKey.characters.size,
+      isBackward: false,
+      isFocused: false,
+      marks: null
+    }
+  )
+
+  return next
+}

test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-void/input.yaml

@@ -0,0 +1,15 @@
+
+nodes:
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: one two
+  - kind: block
+    type: image
+    isVoid: true
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: three

test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void-and-void/output.yaml

@@ -0,0 +1,12 @@
+
+nodes:
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: "one "
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: three

test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void/index.js

@@ -0,0 +1,36 @@
+
+import assert from 'assert'
+
+export default function (state) {
+  const { document, selection } = state
+  const texts = document.getTexts()
+  const first = texts.first()
+  const range = selection.merge({
+    anchorKey: first.key,
+    anchorOffset: 4,
+    focusKey: first.key,
+    focusOffset: 7
+  })
+
+  const next = state
+    .change()
+    .select(range)
+    .delete()
+    .state
+
+  const anchorAndFocusKey = next.document.getTexts().first()
+  assert.deepEqual(
+    next.selection.toJS(),
+    {
+      anchorKey: anchorAndFocusKey.key,
+      anchorOffset: anchorAndFocusKey.characters.size,
+      focusKey: anchorAndFocusKey.key,
+      focusOffset: anchorAndFocusKey.characters.size,
+      isBackward: false,
+      isFocused: false,
+      marks: null
+    }
+  )
+
+  return next
+}

test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void/input.yaml

@@ -0,0 +1,15 @@
+
+nodes:
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: one two
+  - kind: block
+    type: image
+    isVoid: true
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: three

test/changes/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-partially-non-void/output.yaml

@@ -0,0 +1,15 @@
+
+nodes:
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: "one "
+  - kind: block
+    type: image
+    isVoid: true
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: three

test/changes/fixtures/at-current-range/delete/void-inline-as-first-with-non-void-block-next/index.js

@@ -0,0 +1,37 @@
+
+import assert from 'assert'
+
+export default function (state) {
+  const { document, selection } = state
+  const texts = document.getTexts()
+  const firstText = texts.first()
+  const inlineText = texts.get(1)
+  const lastBlockText = texts.get(3)
+  const range = selection.merge({
+    anchorKey: inlineText.key,
+    anchorOffset: 0,
+    focusKey: lastBlockText.key,
+    focusOffset: 0
+  })
+
+  const next = state
+    .change()
+    .select(range)
+    .delete()
+    .state
+  const newFirstText = next.document.getTexts().first()
+  assert.deepEqual(
+    next.selection.toJS(),
+    {
+      anchorKey: newFirstText.key,
+      anchorOffset: firstText.text.length,
+      focusKey: newFirstText.key,
+      focusOffset: firstText.text.length,
+      isBackward: false,
+      isFocused: false,
+      marks: null
+    }
+  )
+
+  return next
+}

test/changes/fixtures/at-current-range/delete/void-inline-as-first-with-non-void-block-next/input.yaml

@@ -0,0 +1,20 @@
+
+nodes:
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: one
+      - kind: inline
+        type: image
+        isVoid: true
+        nodes:
+          - kind: text
+            text: ""
+      - kind: text
+        text: two
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: three

test/changes/fixtures/at-current-range/delete/void-inline-as-first-with-non-void-block-next/output.yaml

@@ -0,0 +1,7 @@
+
+nodes:
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: onethree

test/changes/fixtures/at-current-range/delete/whole-block-with-other-block-type-below/index.js

@@ -0,0 +1,38 @@
+
+import assert from 'assert'
+
+export default function (state) {
+  const { document, selection } = state
+  const texts = document.getTexts()
+  const first = texts.first()
+  const second = texts.get(1)
+  const range = selection.merge({
+    anchorKey: first.key,
+    anchorOffset: 0,
+    focusKey: second.key,
+    focusOffset: 0
+  })
+
+  const next = state
+    .change()
+    .select(range)
+    .delete()
+    .state
+
+  const newFirst = next.document.getTexts().first()
+
+  assert.deepEqual(
+    next.selection.toJS(),
+    {
+      anchorKey: newFirst.key,
+      anchorOffset: 0,
+      focusKey: newFirst.key,
+      focusOffset: 0,
+      isBackward: false,
+      isFocused: false,
+      marks: null
+    }
+  )
+
+  return next
+}

test/changes/fixtures/at-current-range/delete/whole-block-with-other-block-type-below/input.yaml

@@ -0,0 +1,12 @@
+
+nodes:
+  - kind: block
+    type: h1
+    nodes:
+      - kind: text
+        text: "A header"
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: "A paragraph"

test/changes/fixtures/at-current-range/delete/whole-block-with-other-block-type-below/output.yaml

@@ -0,0 +1,7 @@
+
+nodes:
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: "A paragraph"