Skip to content

Conversation

fbcouch
Copy link
Contributor

@fbcouch fbcouch commented Mar 31, 2022

On the iPad, starting with flutter 2.10, the engine sets up a UIIndirectScribbleInteraction and attempts to communicate with the framework over the flutter/textinput channel when a Scribble interaction begins (see flutter/engine#24224). However, before that channel has a method call handler, the engine does not get a reply, so iPadOS continues sending pencil input to the Scribble feature, rather than allowing flutter to capture the input.

Adding a default method call handler allows the engine to get a null reply immediately, so that iPadOS can finish the Scribble interaction and allow input to flow back to flutter. TextInput will set it's own handler once instantiated, so this does not appear to interfere with normal text input.

Fixes #101016

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

…hat TextInput.requestElementsInRect can return immediately, even before TextInput initialization
@flutter-dashboard flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Mar 31, 2022
@jmagman
Copy link
Member

jmagman commented Apr 4, 2022

/cc @justinmc @LongCatIsLooong who reviewed #75472

SystemChannels.system.setMessageHandler((dynamic message) => handleSystemMessage(message as Object));
SystemChannels.lifecycle.setMessageHandler(_handleLifecycleMessage);
SystemChannels.platform.setMethodCallHandler(_handlePlatformMessage);
SystemChannels.textInput.setMethodCallHandler((MethodCall call) => Future<void>.value());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks very odd - and hopefully isn't the real fix here. If it is, this will need a comment explaining what's going on.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With Scribble I think one will be able to enter text when TextInput._instance isn't initialized, so the text input plugin now may start the communication (before that the communication starts when a text field gains focus). So I guess we can do this, or use a separate method channel for Scribble.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I suspect flutter iPad Scribble does not work until the first text field gains focus, because TextInput._instance is lazily initialized?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the issue is that the indirect scribble interaction is set up on the engine side, but with nothing on the framework side handling messages on the textinput channel yet, there are a few moments where iPadOS isn't sure whether a scribble interaction is going to happen or not, so it swallows the inputs and shows a little pencil streak.

If there's a default handler on the method channel, as here, then that doesn't happen because the engine gets a very fast reply over the channel and can let iPadOS know that there won't be a scribble interaction occurring.

I suppose an alternative solution might be to wait to add the UIIndirectScribbleInteraction until we get a message from the framework over the channel...I think that might work in 2.12, since it no longer waits for focus to set up the TextInput._instance.

If this is acceptable, though, I can try to distill that down into something appropriate for a comment.

Copy link
Contributor

@LongCatIsLooong LongCatIsLooong Apr 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that might work in 2.12, since it no longer waits for focus to set up the TextInput._instance.

Ah I see TextInput is accessed in initState.

I would slightly prefer setting up the method handler eagerly over waiting for the first incoming message in the engine ( "first incoming message == we have a framework method channel handler" is kinda of a framework implementation detail). @goderbauer suggestions for making it less odd? Maybe we can expose a TextInput.ensureInitialized method and call that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't fully understand how the scribble feature works, but it is odd that the engine sends us a message and we just drop it on the floor. We should either set up the correct message handler here and handle the message or change the engine side to not send these over the channel until the framework can actually handle them.

@goderbauer
Copy link
Member

(Triage) @fbcouch Do you still have plans to follow up on this PR?

@fbcouch
Copy link
Contributor Author

fbcouch commented Apr 22, 2022

(Triage) @fbcouch Do you still have plans to follow up on this PR?

Yes, thank you for the reminder! I updated the above to add a TextInput.ensureInitialized() call as @LongCatIsLooong suggested, that way we have the proper method channel set up. I suppose it's possible that there could be cases where ScribbleClients get registered without a proper text input on the screen (yet), so I think that's the better way to go, actually.

Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solution looks nice to me compared to the original one, but @LongCatIsLooong should get a final look.

SystemChannels.system.setMessageHandler((dynamic message) => handleSystemMessage(message as Object));
SystemChannels.lifecycle.setMessageHandler(_handleLifecycleMessage);
SystemChannels.platform.setMethodCallHandler(_handlePlatformMessage);
TextInput.ensureInitialized();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any negative impact of this if an app doesn't use text input at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the only negative would be whatever small amount of memory the TextInput instance consumes

Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@p-kuen
Copy link

p-kuen commented May 29, 2022

When does this PR land in stable? It is very frustrating having no clue when this critical issue would be solved in stable. I have no chance to use the stable version right now and have to roll back to an earlier version of flutter to just make it work..
The fix with just a few code line changes is not even available in beta channel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[iOS] Apple Pencil cannot slide and scribble normally after the system's Scribble is turned on
8 participants