Fix loss of negative text selection ranges

[justinmc]

Description

In the framework, changing the value of a TextEditingController to something with a selection of (-1, -1) was coming back from the engine as (length, length). This was because of improper handling of signed and unsigned integers. The -1 was being interpreted as a very large unsigned integer, which was then clamped to the length of the string.

This PR fixes the problem by reading negative selection range values as zero.

Related Issues

Related to flutter/flutter#61282

Tests

I added the following tests:

Replace this with a list of the tests that you added as part of this PR. A
change in behaviour with no test covering it will likely get reverted
accidentally sooner or later. PRs must include tests for all
changed/updated/fixed behaviors. See [testing the engine] for instructions on
writing and running engine tests.

Breaking Change

This is unlikely to break anything unless there is an iOS-only app (Android behaves correctly) that relies on this broken behavior.

[LongCatIsLooong]

I think we can just use the MAX macro instead?

[justinmc]

MIN and MAX appear to cast their inputs to unsigned integers, which is the cause of this bug. I was seeing things like MAX(-1, 0) => 2,147,483,647, and MIN(-1, 0) => 0. This function lets us be sure we're sending sensible unsigned ints to MIN and MAX.

[LongCatIsLooong]

Hmm maybe that's because -1 and 0 are number literals, so their type was inferred to NSUInteger before the comparison took place? I think MAX should work fine if you're converting the number from an NSNumber using intValue.

[justinmc]

Here's a specific example I was able to get while running the app. I saw it at this line:

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

Line 376 in 3c1e336

int start = MIN(MAX(range.location, 0), text.length);

I'm logging like this:

NSLog(@"MAX(%li, 0) => %li, MIN(MAX(range.location, 0), %li) => %li", range.location, MAX(range.location, 0), text.length, MIN(MAX(range.location, 0), text.length));

When range.location is -1 and text.length is 2 this is what I see logged:

MAX(-1, 0) => -1, MIN(MAX(range.location, 0), 2) => 2

But I would expect the MAX to return 0 and the overall MIN to return 0 as well.

[LongCatIsLooong]

I'm not sure what an empty selection stands for. Shouldn't there always be a selected range?

[justinmc]

That confused me too. I'm not sure if there is a use case for that, but it seems to often default to (-1, -1) in the framework. In the case of this bug, for example, the user was setting the text of a TextEditingController, which explicitly sets the selection to (-1, -1) (code). The cursor ends up at the beginning of the field.

[LongCatIsLooong]

On android it translates to Selection.removeSelection(mEditable);. Not sure the same option exists on iOS.

engine/shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java

Lines 369 to 380 in 3c1e336

	private void applyStateToSelection(TextInputChannel.TextEditState state) {
	int selStart = state.selectionStart;
	int selEnd = state.selectionEnd;
	if (selStart >= 0
	&& selStart <= mEditable.length()
	&& selEnd >= 0
	&& selEnd <= mEditable.length()) {
	Selection.setSelection(mEditable, selStart, selEnd);
	} else {
	Selection.removeSelection(mEditable);
	}
	}

[justinmc]

@LongCatIsLooong FYI updated and commented as we discussed, in case you want to take another look before merge.

[LongCatIsLooong]

LGTM 👍