Skip to content

Commit

Permalink
Cherry Pick: [SuperEditor] Fix crashing when deleting all content whi…
Browse files Browse the repository at this point in the history
…ch contains a non-deletable node at an edge (Resolves #2393) (#2405)
  • Loading branch information
github-actions[bot] authored Nov 27, 2024
1 parent 88502ab commit aa4aeb3
Show file tree
Hide file tree
Showing 4 changed files with 1,193 additions and 23 deletions.
35 changes: 30 additions & 5 deletions super_editor/lib/src/default_editor/common_editor_operations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import 'dart:math';
import 'dart:ui';

import 'package:attributed_text/attributed_text.dart';
import 'package:collection/collection.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/services.dart';
import 'package:linkify/linkify.dart';
Expand Down Expand Up @@ -1381,13 +1382,15 @@ class CommonEditorOperations {
/// Returns the [DocumentPosition] where the caret should sit after deleting
/// the given [selection] from the given [document].
///
/// Returns `null` if there are no deletable nodes within the [selection].
///
/// This method doesn't delete any content. Instead, it determines what would
/// be deleted if a delete operation was run for the given [selection]. Based
/// on the shared understanding of content deletion rules, the resulting caret
/// position is returned.
// TODO: Move this method to an appropriate place. It was made public and static
// because document_keyboard_actions.dart also uses this behavior.
static DocumentPosition getDocumentPositionAfterExpandedDeletion({
static DocumentPosition? getDocumentPositionAfterExpandedDeletion({
required Document document,
required DocumentSelection selection,
}) {
Expand Down Expand Up @@ -1419,15 +1422,32 @@ class CommonEditorOperations {
final bottomNodePosition =
baseNodeIndex < extentNodeIndex ? extentPosition.nodePosition : basePosition.nodePosition;

final normalizedRange = selection.normalize(document);
final nodes = document.getNodesInside(normalizedRange.start, normalizedRange.end);
final firstDeletableNodeId = nodes.firstWhereOrNull((node) => node.isDeletable)?.id;

DocumentPosition newSelectionPosition;

if (baseNodeIndex != extentNodeIndex) {
if (topNodePosition == topNode.beginningPosition && bottomNodePosition == bottomNode.endPosition) {
// All nodes in the selection will be deleted. Assume that the start
// node will be retained and converted into a paragraph, if it's not
// All deletable nodes in the selection will be deleted. Assume that one of the
// nodes will be retained and converted into a paragraph, if it's not
// already a paragraph.

final emptyParagraphId = topNode.isDeletable
? topNode.id
: bottomNode.isDeletable
? bottomNode.id
: firstDeletableNodeId;

if (emptyParagraphId == null) {
// There are no deletable nodes in the selection. Fizzle.
// We don't expect this method to be called if there are no deletable nodes.
return null;
}

newSelectionPosition = DocumentPosition(
nodeId: topNode.id,
nodeId: emptyParagraphId,
nodePosition: const TextNodePosition(offset: 0),
);
} else if (topNodePosition == topNode.beginningPosition) {
Expand Down Expand Up @@ -2277,7 +2297,7 @@ class CommonEditorOperations {
/// moves the caret, it's possible that the clipboard content will be pasted
/// at the wrong spot.
void paste() {
DocumentPosition pastePosition = composer.selection!.extent;
DocumentPosition? pastePosition = composer.selection!.extent;

// Start a transaction so that we can capture both the initial deletion behavior,
// and the clipboard content insertion, all as one transaction.
Expand All @@ -2290,6 +2310,11 @@ class CommonEditorOperations {
selection: composer.selection!,
);

if (pastePosition == null) {
// There are no deletable nodes in the selection. Do nothing.
return;
}

// Delete the selected content.
editor.execute([
DeleteContentRequest(documentRange: composer.selection!),
Expand Down
65 changes: 48 additions & 17 deletions super_editor/lib/src/default_editor/multi_node_editing.dart
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,10 @@ class DeleteContentCommand extends EditCommand {
if (endNode == null) {
throw Exception('Could not locate end node for DeleteSelectionCommand: ${normalizedRange.end}');
}
final endNodeIndex = document.getNodeIndexById(endNode.id);

// We expect that this command will only be called when the delete range
// contains at least one deletable node.
final firstDeletableNodeId = nodes.firstWhere((node) => node.isDeletable).id;

executor.logChanges(
_deleteNodesBetweenFirstAndLast(
Expand Down Expand Up @@ -786,19 +789,40 @@ class DeleteContentCommand extends EditCommand {
);
}

final wereAllDeletableNodesInRangeDeleted = nodes.every(
(node) => document.getNodeById(node.id) == null || !node.isDeletable,
);
final hasNonDeletableNodesInRange = nodes.any((node) => !node.isDeletable);

// If all selected nodes were deleted, e.g., the user selected from
// the beginning of the first node to the end of the last node, then
// we need insert an empty paragraph node so that there's a place
// to position the caret.
if (document.getNodeById(startNode.id) == null && document.getNodeById(endNode.id) == null) {
final insertIndex = min(startNodeIndex, endNodeIndex);
if (wereAllDeletableNodesInRangeDeleted) {
// If there are any non-deletable nodes in the range, insert the new node
// after the last non-deletable node. Otherwise, insert the new node at
// the position where the first selected node was.
final insertIndex = hasNonDeletableNodesInRange //
? document.getNodeIndexById(nodes.lastWhere((node) => !node.isDeletable).id) + 1
: startNodeIndex;

// If one of the edge nodes is deletable, we can use it as the ID for the
// new empty paragraph. Otherwise, use the ID of the first deletable node in the range.
// We expect that this method is never called when there are no deletable nodes
// in the range.
final emptyParagraphId = startNode.isDeletable
? startNode.id
: endNode.isDeletable
? endNode.id
: firstDeletableNodeId;

document.insertNodeAt(
insertIndex,
ParagraphNode(id: startNode.id, text: AttributedText()),
ParagraphNode(id: emptyParagraphId, text: AttributedText()),
);
executor.logChanges([
DocumentEdit(
NodeChangeEvent(startNode.id),
NodeChangeEvent(emptyParagraphId),
)
]);
}
Expand Down Expand Up @@ -1144,12 +1168,12 @@ class DeleteSelectionCommand extends EditCommand {
}
}

final newSelectionPosition = CommonEditorOperations.getDocumentPositionAfterExpandedDeletion(
document: document,
selection: selection,
);

final nodes = document.getNodesInside(selection.start, selection.end);
if (nodes.every((node) => !node.isDeletable)) {
// All selected nodes are non-deletable. Do nothing.
return;
}

if (nodes.length == 2) {
final normalizedSelection = selection.normalize(document);
final nodeAbove = document.getNode(normalizedSelection.start)!;
Expand Down Expand Up @@ -1197,19 +1221,26 @@ class DeleteSelectionCommand extends EditCommand {
}
}

executor
..executeCommand(
DeleteContentCommand(
documentRange: selection,
),
)
..executeCommand(
final newSelectionPosition = CommonEditorOperations.getDocumentPositionAfterExpandedDeletion(
document: document,
selection: selection,
);

executor.executeCommand(
DeleteContentCommand(
documentRange: selection,
),
);

if (newSelectionPosition != null) {
executor.executeCommand(
ChangeSelectionCommand(
DocumentSelection.collapsed(position: newSelectionPosition),
SelectionChangeType.deleteContent,
SelectionReason.userInteraction,
),
);
}
}
}

Expand Down
Loading

0 comments on commit aa4aeb3

Please sign in to comment.