Fix empty composing range on iOS #10381
Conversation
|
@cbracken Is this part of the engine tested anywhere? |
|
We should be able to test this using the Scenarios app. You could write a scenario that mocks out the communication from the framework. Unfortunately, testing for an infinite loop regression is .. hard. You could just test that on the first response it's what you expect it to be, and if not, fail immediately I guess. |
|
@dnfield It's not clear to me how I'd trigger text input via the IME in the Scenarios app. That's what's ultimately triggers these messages that I need to check |
|
Just use the channel directly to send the messages that would cause problems |
|
The incorrect message causing the problems was crafted by the engine as response to IME input events. This PR ensures that the correct message is generated and that's what I want to test: In response to an IME input event, the engine sends the right message. If I bypass the message generation code in the engine and just send the correct message directly, it's not really going to be a regression test for this bug. |
|
Ohhh I had it backwards. I'm not sure if we can do something in the iOS unit tests with a mock... But the engine is hard to mock out |
|
|
||
| NSUInteger composingBase = 0; | ||
| NSUInteger composingExtent = 0; | ||
| // Empty compositing range is represented by [TextRange.empty]. |
There was a problem hiding this comment.
Avoid the bracketing around a Dart constant and make it clear you're referring to a framework class here.
[email protected]:flutter/engine.git/compare/dac86eb7fbf7...81f695f git log dac86eb..81f695f --no-merges --oneline 2019-08-07 [email protected] Roll src/third_party/dart f3139f57b4..f29f41f1a5 (3 commits) (flutter/engine#10693) 2019-08-07 [email protected] Roll buildroot forward to unbreak ToT (flutter/engine#10694) 2019-08-07 [email protected] Rolls engine to Android SDK 29 and its corresponding tools (flutter/engine#10692) 2019-08-07 [email protected] Fix empty composing range on iOS (flutter/engine#10381) 2019-08-07 [email protected] Roll src/third_party/skia ed19e97294f6..593290ed75fd (2 commits) (flutter/engine#10691) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff ([email protected]), and stop the roller if necessary.
4 participants
An empty compositing range is represented on Android and within the framework by setting the composing base and extend to -1 (see https://master-api.flutter.dev/flutter/services/TextRange/empty-constant.html). On iOS we incorrectly defined an empty compositing range by setting these values to 0. This can lead to an infinite loop where the engine tells the framework that the compositing range is [0, 0], to which the framework responds that the composing range now is [-1,-1], to which the engine responds that the compositing range is [0,0] and so on.... users were seeing these fights as flickering text in a text field for example.
Fixes flutter/flutter#36971.
Related issue: We really shouldn't be sending NSUIntegers: flutter/flutter#37462