super_editor icon indicating copy to clipboard operation
super_editor copied to clipboard

[BUG] - SuperEditor clears selection when application window loses focus

Open miguelcmedeiros opened this issue 1 year ago • 5 comments

Package Version super_editor main branch (commit https://github.com/superlistapp/super_editor/commit/7eae101b6f1b12956aa1bb348139f50689b61244).

User Info Superlist

To Reproduce Steps to reproduce the behavior:

  1. Go to SuperEditor demo on macOS (or web)
  2. Click on select a word
  3. Move focus to another application (e.g. chrome)
  4. Move focus back to demo app

Actual behavior Selection is cleared.

Expected behavior Selection should be the same as before switching to another application.

Platform macOS and web.

Flutter version master

Screenshots

https://github.com/user-attachments/assets/900f7d87-0398-4ba7-aa44-bda84de84d46

miguelcmedeiros avatar Aug 27 '24 08:08 miguelcmedeiros

Cc @matthew-carroll @angelosilvestre

miguelcmedeiros avatar Aug 27 '24 08:08 miguelcmedeiros

Possibly seems like app lifecycle event being reported wrong on macOS. Looking into it.

knopp avatar Aug 27 '24 13:08 knopp

Nevermind, that was a red herring. Super_editor unfocuses on IME disconnected, but it only refocuses (and thus restores selection) when being interacted or scrolling. So this seems like super_editor issue?

knopp avatar Aug 27 '24 17:08 knopp

@knopp is there a way to identify the situation where the window loses focus and regains it? In the case where the IME connection closes but the focus remains in the current window, I think we probably have the desired behavior. I think it's only when switching between windows that we're seeing an undesirable policy. But I'm not sure where to detect that situation.

Also @miguelcmedeiros experimented with a standard Flutter text field and it appears that in some manner it retains focus between window focus changes. Miguel found that when typing in a regular Flutter text field, it seems to retain caret position when regaining window focus, and also allows for immediate typing through the IME connection.

SuperTextField somehow has a middle ground. When the Flutter window regains focus the SuperTextField regains a selection, but the selection places the caret at the end of the content. Despite the caret, you can't type into the SuperTextField until the user taps on it (probably to give it focus).

We should create a consistent result for window focus change.

matthew-carroll avatar Aug 28 '24 18:08 matthew-carroll

CC @angelosilvestre

matthew-carroll avatar Aug 28 '24 18:08 matthew-carroll

@matthew-carroll @miguelcmedeiros @knopp

This took some debugging to find out what is happening. The FocusManager has a listener to lifecycle events:

void _appLifecycleChange(AppLifecycleState state) {
  if (state == AppLifecycleState.resumed) {
    if (_primaryFocus != rootScope) { <--- this is the relevant condition
      assert(_focusDebug(() => 'focus changed while app was paused, ignoring $_suspendedNode'));
      _suspendedNode = null;
    } else if (_suspendedNode != null) {
      assert(_focusDebug(() => 'requesting focus for $_suspendedNode'));
      _suspendedNode!.requestFocus();
      _suspendedNode = null;
    }
  } else if (_primaryFocus != rootScope) {
    assert(_focusDebug(() => 'suspending $_primaryFocus'));
    _markedForFocus = rootScope;
    _suspendedNode = _primaryFocus;
    applyFocusChangesIfNeeded();
  }
}

This seems to be introduced in https://github.com/flutter/flutter/issues/87061

When the app goes to background, it sets the rootScope (root FocusNode of the app focus tree) as the primary focus node. Then, when the app is resumed, it restores the previous primary focus node, only if the current primary focus is the root focus.

Usually we will have this sequence of events:

  1. "Some focus node" has primary focus
  2. The app goes to background
  3. rootScope receives primary focus
  4. App is resumed
  5. Primary focus is still rootScope
  6. "Some focus node" receives primary focus

The problem in SuperEditor is that, on the editor toolbar, we have this code when we hide the toolbar:

// Ensure that focus returns to the editor.
//
// I tried explicitly unfocus()'ing the URL textfield
// in the toolbar but it didn't return focus to the
// editor. I'm not sure why.
 _editorFocusNode.requestFocus();

I suppose SuperList has a similar code. This code runs when the app goes to background and it ends up modifying the sequence of events:

  1. "Some focus node" has primary focus
  2. The app goes to background
  3. rootScope receives primary focus
  4. _editorFocusNode receives primary focus
  5. App is resumed
  6. Primary focus is not rootScope
  7. _editorFocusNode DOES NOT receive primary focus

We could modify the code where we request focus to check if the primary code is the rootScope and avoid requesting focus in that case:

if (FocusManager.instance.primaryFocus != FocusManager.instance.rootScope) {
  _editorFocusNode.requestFocus();
}

This seems to make SuperEditor work as expected.

angelosilvestre avatar Dec 18 '24 21:12 angelosilvestre

@angelosilvestre Thanks for the analysis. That solved the problem for Superlist as well.

miguelcmedeiros avatar Jan 07 '25 10:01 miguelcmedeiros

@angelosilvestre can we make similar modifications to our example code, along with code comments explaining why?

matthew-carroll avatar Jan 08 '25 17:01 matthew-carroll