Fix inconsistencies when calculating start/end handle rects for selection handles #87236
Conversation
…he current frame if not fall back
flutter-dashboard
bot
added
the
framework
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
There was a problem hiding this comment.
Confirmed that this fixes the assertions seen when testing #85789 with the SkParagraph text renderer
| // for a collapsed selection, fall back to this case when that happens. | ||
| firstSelectedGraphemeExtent = 0; | ||
| lastSelectedGraphemeExtent = 0; | ||
| startHandleRect = null; |
There was a problem hiding this comment.
This can be removed - startHandleRect/endHandleRect will be initialized to null by default
There was a problem hiding this comment.
Do you mean I should initialize them to null by default, or is this done automatically. I tried removing this else block but got some errors saying that I cannot use the rects before they have been initialized.
There was a problem hiding this comment.
The variables will be initialized to null by default if they are not final. If they are final, then they must be explicitly assigned.
Either way is fine.
There was a problem hiding this comment.
LGTM 👍 . Thanks @jason-simmons for finding this.
Is it possible to test that the asserts are no longer happening?
flutter-dashboard
bot
added
the
f: cupertino
Description
This change updates the calculation for start/end rects for selection handles to use the current frames selection, when the current frames text is the same as the previous text, if not then we fall back to
widget.renderObject.preferredLineHeightfor the calculation. This is becausewidget.renderObject.getRectForComposingRangeis only available for the previous frame, so if the current frames text and previous differ thengetRectForComposingRangecould fail.In addition this change also removes the asserts for
startHandleRectandendHandleRectsince_TextSelectionHandleOverlayState.buildchecks for null handle rects.Related Issues
Tests
Added test to make sure we are falling back to
preferredLineHeight.Pre-launch Checklist
///).