Fix bug where void nodes would not deleted, even though they were selected (#1041)

src/transforms/at-range.js

@@ -68,15 +68,7 @@ Transforms.deleteAtRange = (transform, range, options = {}) => {
   if (range.isCollapsed) return
 
   const { normalize = true } = options
-  const { startKey, startOffset, endKey, endOffset } = range
+  let { startKey, startOffset, endKey, endOffset } = range
-
-  // If the start and end key are the same, we can just remove text.
-  if (startKey == endKey) {
-    const index = startOffset
-    const length = endOffset - startOffset
-    transform.removeTextByKey(startKey, index, length, { normalize })
-    return
-  }
 
   // Split at the range edges within a common ancestor, without normalizing.
   let { state } = transform
@@ -84,9 +76,83 @@ Transforms.deleteAtRange = (transform, range, options = {}) => {
   let ancestor = document.getCommonAncestor(startKey, endKey)
   let startChild = ancestor.getFurthestAncestor(startKey)
   let endChild = ancestor.getFurthestAncestor(endKey)
+
+  // If the start child is a void node, and the range begins or
+  // ends (when range is backward) at the start of it, remove it
+  // and set nextSibling as startChild until there is no startChild
+  // that is a void node and included in the selection range
+  let startChildIncludesVoid = startChild.isVoid && (
+    range.anchorOffset === 0 && !range.isBackward ||
+    range.focusOffset === 0 && range.isBackward
+  )
+  while (startChildIncludesVoid) {
+    const nextSibling = document.getNextSibling(startChild.key)
+    transform.removeNodeByKey(startChild.key, OPTS)
+    // Abort if no nextSibling or we are about to process the endChild which is aslo a void node
+    if (!nextSibling || endChild.key === nextSibling.key && nextSibling.isVoid) {
+      startChildIncludesVoid = false
+      return
+    }
+    // Process the next void
+    if (nextSibling.isVoid) {
+      startChild = nextSibling
+    }
+    // Set the startChild, startKey and startOffset in the beginning of the next non void sibling
+    if (!nextSibling.isVoid) {
+      startChild = nextSibling
+      if (startChild.getTexts) {
+        startKey = startChild.getTexts().first().key
+      } else {
+        startKey = startChild.key
+      }
+      startOffset = 0
+      startChildIncludesVoid = false
+    }
+  }
+
+  // If the start child is a void node, and the range ends or
+  // begins (when range is backward) at the end of it move to nextSibling
+  const startChildEndOfVoid = startChild.isVoid && (
+    range.anchorOffset === 1 && !range.isBackward ||
+    range.focusOffset === 1 && range.isBackward
+  )
+  if (startChildEndOfVoid) {
+    const nextSibling = document.getNextSibling(startChild.key)
+    if (nextSibling) {
+      startChild = nextSibling
+      if (startChild.getTexts) {
+        startKey = startChild.getTexts().first().key
+      } else {
+        startKey = startChild.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
+    if (ancestor.isVoid) {
+      // Deselect if this is the only node left in document
+      if (document.nodes.size === 1) {
+        transform.deselect()
+      }
+      transform.removeNodeByKey(ancestor.key, OPTS)
+      return
+    }
+    // Remove the text
+    const index = startOffset
+    const length = endOffset - startOffset
+    transform.removeTextByKey(startKey, index, length, { normalize })
+    return
+  }
+
+  // Split at the range edges within a common ancestor, without normalizing.
+  ancestor = document.getCommonAncestor(startKey, endKey)
+  startChild = ancestor.getFurthestAncestor(startKey)
+  endChild = ancestor.getFurthestAncestor(endKey)
   const startOff = (startChild.kind == 'text' ? 0 : startChild.getOffset(startKey)) + startOffset
   const endOff = (endChild.kind == 'text' ? 0 : endChild.getOffset(endKey)) + endOffset
-
   transform.splitNodeByKey(startChild.key, startOff, OPTS)
   transform.splitNodeByKey(endChild.key, endOff, OPTS)
 
@@ -107,11 +173,18 @@ Transforms.deleteAtRange = (transform, range, options = {}) => {
     })
   }
 
-  // If the start and end block are different, move all of the nodes from the
+
-  // end block into the start block.
   const startBlock = document.getClosestBlock(startKey)
   const endBlock = document.getClosestBlock(document.getNextText(endKey).key)
 
+  // If the endBlock is void, just remove the startBlock
+  if (endBlock.isVoid) {
+    transform.removeNodeByKey(startBlock.key)
+    return
+  }
+
+  // If the start and end block are different, move all of the nodes from the
+  // end block into the start block
   if (startBlock.key !== endBlock.key) {
     endBlock.nodes.forEach((child, i) => {
       const newKey = startBlock.key

test/transforms/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-only-non-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: 0,
+    focusKey: second.key,
+    focusOffset: 0
+  })
+
+  const next = state
+    .transform()
+    .select(range)
+    .delete()
+    .apply()
+
+  const anchorAndFocusKey = next.document.getTexts().first()
+  assert.deepEqual(
+    next.selection.toJS(),
+    {
+      anchorKey: anchorAndFocusKey.key,
+      anchorOffset: 0,
+      focusKey: anchorAndFocusKey.key,
+      focusOffset: 0,
+      isBackward: false,
+      isFocused: false,
+      marks: null
+    }
+  )
+
+  return next
+}

test/transforms/fixtures/at-current-range/delete/non-void-block-as-first-with-void-siblings-only-non-void/input.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: two

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

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

test/transforms/fixtures/at-current-range/delete/void-block-as-first-with-non-void-siblings-all/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 last = texts.last()
+  const range = selection.merge({
+    anchorKey: first.key,
+    anchorOffset: 0,
+    focusKey: last.key,
+    focusOffset: 3
+  })
+
+  const next = state
+    .transform()
+    .select(range)
+    .delete()
+    .apply()
+
+  assert.deepEqual(
+    next.selection.toJS(),
+    {
+      anchorKey: second.key,
+      anchorOffset: 0,
+      focusKey: second.key,
+      focusOffset: 0,
+      isBackward: false,
+      isFocused: false,
+      marks: null
+    }
+  )
+
+  return next
+}

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

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

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

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

test/transforms/fixtures/at-current-range/delete/void-block-as-first-with-non-void-siblings-backward-selection-all/index.js

@@ -0,0 +1,39 @@
+
+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 last = texts.last()
+
+  const range = selection.merge({
+    anchorKey: last.key,
+    anchorOffset: 3,
+    focusKey: first.key,
+    focusOffset: 0,
+    isBackward: true
+  })
+
+  const next = state
+    .transform()
+    .select(range)
+    .delete()
+    .apply()
+
+  assert.deepEqual(
+    next.selection.toJS(),
+    {
+      anchorKey: second.key,
+      anchorOffset: 0,
+      focusKey: second.key,
+      focusOffset: 0,
+      isBackward: false,
+      isFocused: false,
+      marks: null
+    }
+  )
+
+  return next
+}

test/transforms/fixtures/at-current-range/delete/void-block-as-first-with-non-void-siblings-backward-selection-all/input.yaml

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

test/transforms/fixtures/at-current-range/delete/void-block-as-first-with-non-void-siblings-backward-selection-all/output.yaml

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

test/transforms/fixtures/at-current-range/delete/void-block-as-first-with-non-void-siblings-only-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 second = texts.get(1)
+  const range = selection.merge({
+    anchorKey: first.key,
+    anchorOffset: 0,
+    focusKey: second.key,
+    focusOffset: 0
+  })
+
+  const next = state
+    .transform()
+    .select(range)
+    .delete()
+    .apply()
+
+  assert.deepEqual(
+    next.selection.toJS(),
+    {
+      anchorKey: second.key,
+      anchorOffset: 0,
+      focusKey: second.key,
+      focusOffset: 0,
+      isBackward: false,
+      isFocused: false,
+      marks: null
+    }
+  )
+
+  return next
+}

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

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

test/transforms/fixtures/at-current-range/delete/void-block-as-first-with-non-void-siblings-only-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: two

test/transforms/fixtures/at-current-range/delete/void-block-as-first-with-non-void-siblings-select-void-and-next-word/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 second = texts.get(1)
+  const range = selection.merge({
+    anchorKey: first.key,
+    anchorOffset: 0,
+    focusKey: second.key,
+    focusOffset: 4
+  })
+
+  const next = state
+    .transform()
+    .select(range)
+    .delete()
+    .apply()
+
+  assert.deepEqual(
+    next.selection.toJS(),
+    {
+      anchorKey: second.key,
+      anchorOffset: 0,
+      focusKey: second.key,
+      focusOffset: 0,
+      isBackward: false,
+      isFocused: false,
+      marks: null
+    }
+  )
+
+  return next
+}

test/transforms/fixtures/at-current-range/delete/void-block-as-first-with-non-void-siblings-select-void-and-next-word/input.yaml

@@ -0,0 +1,18 @@
+
+nodes:
+  - kind: block
+    type: image
+    isVoid: true
+    nodes:
+      - kind: text
+        text: ""
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: some words
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: other words

test/transforms/fixtures/at-current-range/delete/void-block-as-first-with-non-void-siblings-select-void-and-next-word/output.yaml

@@ -0,0 +1,12 @@
+
+nodes:
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: " words"
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: other words

test/transforms/fixtures/at-current-range/delete/void-block-as-first-with-non-void-siblings-select-void-end-and-next-word/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 second = texts.get(1)
+  const range = selection.merge({
+    anchorKey: first.key,
+    anchorOffset: 1,
+    focusKey: second.key,
+    focusOffset: 5
+  })
+
+  const next = state
+    .transform()
+    .select(range)
+    .delete()
+    .apply()
+
+  assert.deepEqual(
+    next.selection.toJS(),
+    {
+      anchorKey: first.key,
+      anchorOffset: 1,
+      focusKey: first.key,
+      focusOffset: 1,
+      isBackward: false,
+      isFocused: false,
+      marks: null
+    }
+  )
+
+  return next
+}

test/transforms/fixtures/at-current-range/delete/void-block-as-first-with-non-void-siblings-select-void-end-and-next-word/input.yaml

@@ -0,0 +1,18 @@
+
+nodes:
+  - kind: block
+    type: image
+    isVoid: true
+    nodes:
+      - kind: text
+        text: ""
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: some words
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: other words

test/transforms/fixtures/at-current-range/delete/void-block-as-first-with-non-void-siblings-select-void-end-and-next-word/output.yaml

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

test/transforms/fixtures/at-current-range/delete/void-block-as-only/index.js

@@ -0,0 +1,35 @@
+
+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: 0,
+    focusKey: first.key,
+    focusOffset: 1
+  })
+
+  const next = state
+    .transform()
+    .select(range)
+    .delete()
+    .apply()
+
+  assert.deepEqual(
+    next.selection.toJS(),
+    {
+      anchorKey: null,
+      anchorOffset: 0,
+      focusKey: null,
+      focusOffset: 0,
+      isBackward: false,
+      isFocused: false,
+      marks: null
+    }
+  )
+
+  return next
+}

test/transforms/fixtures/at-current-range/delete/void-block-as-only/input.yaml

@@ -0,0 +1,8 @@
+
+nodes:
+  - kind: block
+    type: image
+    isVoid: true
+    nodes:
+      - kind: text
+        text: ""

test/transforms/fixtures/at-current-range/delete/void-block-as-only/output.yaml

@@ -0,0 +1,2 @@
+
+nodes: []

test/transforms/fixtures/at-current-range/delete/void-blocks-as-first-with-non-void-siblings-only-first-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 second = texts.get(1)
+  const range = selection.merge({
+    anchorKey: first.key,
+    anchorOffset: 0,
+    focusKey: second.key,
+    focusOffset: 0
+  })
+
+  const next = state
+    .transform()
+    .select(range)
+    .delete()
+    .apply()
+
+  assert.deepEqual(
+    next.selection.toJS(),
+    {
+      anchorKey: second.key,
+      anchorOffset: 0,
+      focusKey: second.key,
+      focusOffset: 0,
+      isBackward: false,
+      isFocused: false,
+      marks: null
+    }
+  )
+
+  return next
+}

test/transforms/fixtures/at-current-range/delete/void-blocks-as-first-with-non-void-siblings-only-first-void/input.yaml

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

test/transforms/fixtures/at-current-range/delete/void-blocks-as-first-with-non-void-siblings-only-first-void/output.yaml

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

test/transforms/fixtures/at-current-range/delete/void-blocks-as-first-with-non-void-siblings-only-voids/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 third = texts.get(2)
+  const range = selection.merge({
+    anchorKey: first.key,
+    anchorOffset: 0,
+    focusKey: third.key,
+    focusOffset: 0
+  })
+
+  const next = state
+    .transform()
+    .select(range)
+    .delete()
+    .apply()
+
+  assert.deepEqual(
+    next.selection.toJS(),
+    {
+      anchorKey: third.key,
+      anchorOffset: 0,
+      focusKey: third.key,
+      focusOffset: 0,
+      isBackward: false,
+      isFocused: false,
+      marks: null
+    }
+  )
+
+  return next
+}

test/transforms/fixtures/at-current-range/delete/void-blocks-as-first-with-non-void-siblings-only-voids/input.yaml

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

test/transforms/fixtures/at-current-range/delete/void-blocks-as-first-with-non-void-siblings-only-voids/output.yaml

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

test/transforms/fixtures/at-current-range/delete/void-inline-as-first-with-non-void-sibling/index.js

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

test/transforms/fixtures/at-current-range/delete/void-inline-as-first-with-non-void-sibling/input.yaml

@@ -0,0 +1,13 @@
+
+nodes:
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: inline
+        type: image
+        isVoid: true
+        nodes:
+          - kind: text
+            text: ""
+      - kind: text
+        text: abc

test/transforms/fixtures/at-current-range/delete/void-inline-as-first-with-non-void-sibling/output.yaml

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