[iOSTextInputPlugin] bypass UIKit floating cursor coordinates clamping #26486

LongCatIsLooong · 2021-05-29T07:41:51Z

The current implementation "works" only because UIKit doesn't properly handle "nil" somewhere: flutter/flutter#70267 (comment), and it breaks once the keyboard adds marked text to the input field.

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 and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.
The reviewer has submitted any presubmit flakes in this PR using the engine presubmit flakes form before re-triggering the failure.

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

…or-fix

LongCatIsLooong · 2021-05-29T20:36:08Z

shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm

+    // This makes sure UITextSelectionView.interactionAssistant is not nil so
+    // UITextSelectionView has access to this view (and its bounds). Otherwise
+    // floating cursor breaks: https://github.com/flutter/flutter/issues/70267.
+    if (@available(iOS 13.0, *)) {


I tested on an iOS 12 simulator and couldn't reproduce the issue.

LongCatIsLooong · 2021-05-29T20:38:33Z

shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m

@@ -79,7 +79,6 @@ - (void)setUp {
 }

 - (void)tearDown {
-  [engine stopMocking];


This line caused all the tests in the file to fail. Did we upgrade ocmock or something?

Nope we didn't. You shouldn't remove this line. The tests are running as part of CI so we know this works before the changes here. Let me know what errors you are getting and I can try to help.

I put it back it starts failing again: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket.appspot.com/8845604182114825824/+/u/Host_Tests_for_ios_debug_sim/stdout

/opt/s/w/ir/cache/builder/src/out/ios_debug_sim/../../flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm:1211: error: -[FlutterTextInputPluginTest testPasswordAutofillHack] : failed: caught "NSInternalInconsistencyException", "** Cannot use mock object OCClassMockObject(FlutterEngine) at 0x600002eabb60. This error usually occurs when a mock object is used after stopMocking has been called on it. In most cases it is not necessary to call stopMocking. If you know you have to, please make sure that the mock object is not used afterwards."

This means you are leaking an object beyond your test that is talking to the mock engine, probably a view controller. Let me look at your test code to see if I can spot it. (this can cause other tests to fail beyond yours)

Looks like your test is fine. One of 2 things are happening:

Your test is messing up the timing and surfacing an issue that was already present where we weren't correctly synchronizing some asynchronous operations (like NSNotifications)

You've introduced a leak that is causing view controllers not to be deallocated, which causes them to leak into other tests and break them.

Here's an example where I had to fix similar issues:
27bf114#diff-9df3e6878a0f23383e1f2de991386935e435c5b77c863de75bd67a4ced38591dR136

I'd try running that test / writing one similar to make sure that the view controller is being deleted. If you run that one test and it fails, you'll know you introduced a leak. Otherwise tracking down the timing issue will take a bit more work.

Turns out UIKit is keeping some of the input views alive and modifying the content of these views even they're removed from the view hierarchy. #26547

gaaclarke

Code looks good. We just have to fix your tests so you don't call stopMocking.

chinmaygarde · 2021-06-03T22:00:02Z

@gaaclarke Looks like you review comments were addressed. Another pass?

LongCatIsLooong · 2021-06-03T22:05:13Z

@chinmaygarde I chose to address a problem in a different PR: #26547. It's currently being reviewed and I think we'll come back to this PR once that problem is fixed.

…or-fix

gaaclarke

LGTM!

gaaclarke · 2021-06-04T16:22:52Z

shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm

 // not have access to its FlutterTextInputDelegate any more.
- (void)decommision {
+- (void)decommission {


… clamping (flutter/engine#26486)

flutter#26486)

LongCatIsLooong added 2 commits May 29, 2021 00:29

[iOSTextInputPlugin] bypass UIKit floating cursor coordinates clamping

761aebc

[iOSTextInputPlugin] bypass UIKit floating cursor coordinates clamping

d19cb61

flutter-dashboard bot added the platform-ios label May 29, 2021

google-cla bot added the cla: yes label May 29, 2021

LongCatIsLooong force-pushed the ios-floating-cursor-fix branch 2 times, most recently from ea93e35 to 3a10d3f Compare May 29, 2021 12:06

Merge remote-tracking branch 'upstream/master' into ios-floating-curs…

39e8eaa

…or-fix

LongCatIsLooong force-pushed the ios-floating-cursor-fix branch from 3a10d3f to 39e8eaa Compare May 29, 2021 12:07

LongCatIsLooong requested review from gaaclarke and justinmc May 29, 2021 20:09

LongCatIsLooong commented May 29, 2021

View reviewed changes

gaaclarke suggested changes Jun 1, 2021

View reviewed changes

add stopMocking back

b13caac

LongCatIsLooong mentioned this pull request Jun 2, 2021

[iOSTextInput] fix potential dangling pointer access #26547

Merged

9 tasks

Merge remote-tracking branch 'upstream/master' into ios-floating-curs…

fa1e498

…or-fix

LongCatIsLooong force-pushed the ios-floating-cursor-fix branch from 459fd32 to fa1e498 Compare June 4, 2021 05:07

fix typo

659e74a

LongCatIsLooong force-pushed the ios-floating-cursor-fix branch from 2e3485a to 659e74a Compare June 4, 2021 08:33

gaaclarke approved these changes Jun 4, 2021

View reviewed changes

LongCatIsLooong added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Jun 4, 2021

fluttergithubbot merged commit 8828a63 into flutter:master Jun 4, 2021

LongCatIsLooong deleted the ios-floating-cursor-fix branch June 4, 2021 19:37

engine-flutter-autoroll mentioned this pull request Jun 6, 2021

Roll Engine from 03d645e782db to 66068e2e7dd9 (20 revisions) flutter/flutter#84083

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 6, 2021

8828a63 [iOSTextInputPlugin] bypass UIKit floating cursor coordinates…

5395990

… clamping (flutter/engine#26486)

naudzghebre pushed a commit to naudzghebre/engine that referenced this pull request Sep 2, 2021

[iOSTextInputPlugin] bypass UIKit floating cursor coordinates clamping (

3101264

flutter#26486)

jmagman mentioned this pull request Sep 20, 2021

Assertion at -[FlutterTextInputView beginFloatingCursorAtPoint:]: 'Another floating cursor is currently active.' flutter/flutter#90416

Closed

LongCatIsLooong mentioned this pull request Apr 26, 2022

[iOS] Add UITextInteraction only when inputDelegate is nil #32881

Merged

8 tasks

jmagman mentioned this pull request Oct 19, 2022

iPhone floating cursor selection #36643

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[iOSTextInputPlugin] bypass UIKit floating cursor coordinates clamping #26486

[iOSTextInputPlugin] bypass UIKit floating cursor coordinates clamping #26486

Uh oh!

LongCatIsLooong commented May 29, 2021

Uh oh!

LongCatIsLooong May 29, 2021

Uh oh!

LongCatIsLooong May 29, 2021

Uh oh!

gaaclarke Jun 1, 2021

Uh oh!

LongCatIsLooong Jun 1, 2021

Uh oh!

gaaclarke Jun 1, 2021 •

edited

Loading

Uh oh!

gaaclarke Jun 1, 2021 •

edited

Loading

Uh oh!

LongCatIsLooong Jun 2, 2021

Uh oh!

gaaclarke left a comment

Uh oh!

chinmaygarde commented Jun 3, 2021

Uh oh!

LongCatIsLooong commented Jun 3, 2021

Uh oh!

gaaclarke left a comment

Uh oh!

gaaclarke Jun 4, 2021

Uh oh!

Uh oh!

[iOSTextInputPlugin] bypass UIKit floating cursor coordinates clamping #26486

[iOSTextInputPlugin] bypass UIKit floating cursor coordinates clamping #26486

Uh oh!

Conversation

LongCatIsLooong commented May 29, 2021

Pre-launch Checklist

Uh oh!

LongCatIsLooong May 29, 2021

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong May 29, 2021

Choose a reason for hiding this comment

Uh oh!

gaaclarke Jun 1, 2021

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong Jun 1, 2021

Choose a reason for hiding this comment

Uh oh!

gaaclarke Jun 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gaaclarke Jun 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong Jun 2, 2021

Choose a reason for hiding this comment

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

chinmaygarde commented Jun 3, 2021

Uh oh!

LongCatIsLooong commented Jun 3, 2021

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

gaaclarke Jun 4, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gaaclarke Jun 1, 2021 •

edited

Loading

gaaclarke Jun 1, 2021 •

edited

Loading