Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android - Creating a non-text node and selecting it after the text with OS IME suggestions on the keyboard causes an exception. #2070

Open
rafalplonka opened this issue Jun 6, 2024 · 6 comments
Assignees

Comments

@rafalplonka
Copy link

Package Version
super_editor, GitHub, stable branch

To Reproduce

  1. Place a caret on the text with keyboard suggestions on Android.
  2. Programmatically create a new node that is not a text node.
  3. Programmatically add a selection on the added node.

Minimal Reproduction Code

This issue only occurs on Android when there is already text that has suggestions (see the video for details). Adding the node doesn't cause a crash, but selecting it afterwards does.

The issue cannot be reproduced in the demo since the demo doesn’t allow for adding a node, such as a divider, while having the caret on the text element.

  editor.execute([
    InsertNodeAfterNodeRequest(existingNodeId: afterNodeId, newNode: node),
    ChangeSelectionRequest(
      DocumentSelection.collapsed(
        position: DocumentPosition(
          nodeId: node.id,
          nodePosition: const UpstreamDownstreamNodePosition.downstream(),
        ),
      ),
      SelectionChangeType.placeCaret,
      SelectionReason.userInteraction,
    )
  ]);

Actual behavior
Crash


Exception: No such document position in the IME content: [DocumentPosition] - node: "edfa8ce7-9b4d-4366-8b5b-fd1e8871bb2c", position: (TextPosition(offset: 0, affinity: TextAffinity.downstream))

When the exception was thrown, this was the stack: 
#0      DocumentImeSerializer._documentToImePosition (package:super_editor/src/default_editor/document_ime/document_serialization.dart:351:7)
#1      DocumentImeSerializer.documentToImeRange (package:super_editor/src/default_editor/document_ime/document_serialization.dart:335:30)
#2      DocumentImeSerializer.toTextEditingValue (package:super_editor/src/default_editor/document_ime/document_serialization.dart:382:32)
#3      DocumentImeInputClient._sendDocumentToIme (package:super_editor/src/default_editor/document_ime/document_ime_communication.dart:273:58)
#4      DocumentImeInputClient._onContentChange (package:super_editor/src/default_editor/document_ime/document_ime_communication.dart:87:5)
#5      ChangeNotifier.notifyListeners (package:flutter/src/foundation/change_notifier.dart:433:24)
#6      ValueNotifier.value= (package:flutter/src/foundation/change_notifier.dart:555:5)
#7      PausableValueNotifier.resumeNotifications (package:super_editor/src/infrastructure/pausable_value_notifier.dart:49:11)

Expected behavior
Horizontal ruler is added, selection is changed to horizontal ruler node

Platform
Android

Flutter version
Flutter v. 3.22.1

Screenshots
https://github.com/superlistapp/super_editor/assets/18536122/51895511-c15d-4ff5-bff4-bd594e97200e

@rafalplonka rafalplonka added the type_bug Something isn't working label Jun 6, 2024
@rafalplonka rafalplonka changed the title Android - Creating a non-text node and selecting it after the text with OS IME suggestions on the keyboard causes a crash. Android - Creating a non-text node and selecting it after the text with OS IME suggestions on the keyboard causes an exception. Jun 6, 2024
@matthew-carroll
Copy link
Contributor

@snowb1shop your issue description mentions two programmatic steps.

Please include a minimal, runnable code sample that demonstrates this problem.

@rafalplonka
Copy link
Author

Hey, thanks for the answer. 

I’ve forked the supereditor package from the main branch and created a demo. 

The commit is really small: 1510658


Only creating a horizontal rule node and placing the selection on it.

To find the issue run the example's main.dart demo on Android, place the caret on the paragraph text, start writing, see the suggestions from the keyboard and then click to create a horizontal rule node from the toolbar.

Link to the fork: https://github.com/snowb1shop/super_editor


void createHr() {
    final selectedNode =
        document.getNodeById(composer.selection!.extent.nodeId)! as TextNode;

    final newNode = HorizontalRuleNode(
      id: Editor.createNodeId(),
    );

    editor.execute([
      InsertNodeAfterNodeRequest(
          existingNodeId: selectedNode.id, newNode: newNode),
      ChangeSelectionRequest(
        DocumentSelection.collapsed(
          position: DocumentPosition(
            nodeId: newNode.id,
            nodePosition: const UpstreamDownstreamNodePosition.downstream(),
          ),
        ),
        SelectionChangeType.placeCaret,
        SelectionReason.userInteraction,
      )
    ]);
  }

@matthew-carroll
Copy link
Contributor

@snowb1shop we ask for self-contained runnable reproductions. It's a friction to pull down an entire fork of the repo just to see the bug. Can you please create a minimal runnable entrypoint with just enough code to recreate the issue and post that as a code block in this issue?

@rafalplonka
Copy link
Author

Here is the updated demo:

  1. Place caret on the text
  2. Click the ElevatedButton
import 'package:flutter/material.dart';
import 'package:super_editor/super_editor.dart';

void main() {
  runApp(
    MaterialApp(
      home: Scaffold(
        body: _StandardEditor(),
      ),
      debugShowCheckedModeBanner: false,
    ),
  );
}

class _StandardEditor extends StatefulWidget {
  const _StandardEditor();

  @override
  State<_StandardEditor> createState() => _StandardEditorState();
}

class _StandardEditorState extends State<_StandardEditor> {
  final GlobalKey _docLayoutKey = GlobalKey();

  late MutableDocument _doc;
  late MutableDocumentComposer _composer;
  late Editor _docEditor;

  late FocusNode _editorFocusNode;

  late ScrollController _scrollController;

  @override
  void initState() {
    super.initState();
    _doc = MutableDocument(nodes: [
      ParagraphNode(
        id: Editor.createNodeId(),
        text: AttributedText(
          "Place a caret on the text",
        ),
      )
    ]);
    _composer = MutableDocumentComposer();
    _docEditor =
        createDefaultDocumentEditor(document: _doc, composer: _composer);
    _editorFocusNode = FocusNode();
    _scrollController = ScrollController();
  }

  @override
  void dispose() {
    _scrollController.dispose();
    _editorFocusNode.dispose();
    _composer.dispose();
    super.dispose();
  }

  @override
  Widget build(BuildContext context) {
    return SafeArea(
      child: Container(
        margin: EdgeInsets.only(top: 40),
        child: Column(
          children: [
            ElevatedButton(
              onPressed: () {
                final selectedNode = _doc.getNodeAt(0)! as TextNode;

                final newNode = HorizontalRuleNode(
                  id: Editor.createNodeId(),
                );

                _docEditor.execute([
                  InsertNodeAfterNodeRequest(
                      existingNodeId: selectedNode.id, newNode: newNode),
                  ChangeSelectionRequest(
                    DocumentSelection.collapsed(
                      position: DocumentPosition(
                        nodeId: newNode.id,
                        nodePosition:
                            const UpstreamDownstreamNodePosition.downstream(),
                      ),
                    ),
                    SelectionChangeType.placeCaret,
                    SelectionReason.userInteraction,
                  )
                ]);
              },
              child: const Text('Create HorizontalRuleNode'),
            ),
            SuperEditor(
              editor: _docEditor,
              document: _doc,
              composer: _composer,
              focusNode: _editorFocusNode,
              scrollController: _scrollController,
              documentLayoutKey: _docLayoutKey,
            ),
          ],
        ),
      ),
    );
  }
}

@angelosilvestre
Copy link
Collaborator

@snowb1shop @matthew-carroll This happens because, on Android, when we tap at a word, Android generates a composing region for that word. In the sample code, the selection is being changed to a non-text node, but we keep the composing region of the previously selected word. As a result, we are trying to apply an invalid composing region.

Adding a ClearComposingRegionRequest to the request list fixes the issue.

In the places that SuperEditor itself issues a ChangeSelectionRequest, we also issue a ClearComposingRegionRequest. Maybe we could change ChangeSelectionCommand to also clear the composing region if the selection change from one node to another. Any thoughts on that?

@rafalplonka
Copy link
Author

Thanks, it worked after adding what you suggested.

In my opinion it makes sense to include it in the ChangeSelectionCommand, especially that it worked correctly on other platforms (iOS, Web).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants