Gracefully handle negative position in getWordAtOffset#128464
Gracefully handle negative position in getWordAtOffset#128464justinmc merged 5 commits intoflutter:masterfrom
Conversation
github-actions
bot
added
the
framework
justinmc
force-pushed
the
negative-position
branch
from
ec8bbc2 to
a0b2b9d
Compare
|
Looking at the Oops I read it wrong maybe it will, it also uses |
| // the position. | ||
| if (TextLayoutMetrics.isWhitespace(plainText.codeUnitAt(effectiveOffset)) | ||
| && effectiveOffset > 0) { | ||
| if (effectiveOffset > 0 |
There was a problem hiding this comment.
If there's no clear repro, could you add an assert here so we could debug what happened?
There was a problem hiding this comment.
I worry that there is a legit case where effectiveOffset can be less than 0, and the correct course of action is just to skip this if. Should I leave it with no assertion so that developers aren't confused if they trigger it in a legitimate case?
There was a problem hiding this comment.
So the theory is that _textPainter.getPositionForOffset is producing a TextPosition of (0, upstream) ? That's not supposed to be a valid output of the getPositionForOffset method I guess. If that's the case this uncovers an upstream bug so I think it's still nice to have an assert here. It won't be an issue in release builds unlike the OOB crash.
There was a problem hiding this comment.
Good point, done. Yeah that's my best guess at what's happening.
|
@Renzo-Olivares I thought I could get it by using a grapheme cluster based on that code but I still never saw |
flutter/flutter@c40baf4...042c036 2023-06-22 [email protected] Remove unnecessary variable `_hasPrimaryFocus` (flutter/flutter#129066) 2023-06-22 [email protected] Fix Material 3 Scrollable `TabBar` (flutter/flutter#125974) 2023-06-22 [email protected] Fix: Closing bottom sheet and removing FAB cause assertion failure (flutter/flutter#128566) 2023-06-22 [email protected] Add `InputDecorationTheme.merge` (flutter/flutter#129011) 2023-06-22 [email protected] Roll Packages from 9af50d4 to 95bc1c6 (6 revisions) (flutter/flutter#129351) 2023-06-22 [email protected] Prevent crashes on range errors when selecting device (flutter/flutter#129290) 2023-06-22 [email protected] Revert "Roll Flutter Engine from 703c9a14ac7f to 8cc6d6d5efdb (1 revision)" (flutter/flutter#129353) 2023-06-22 [email protected] Roll Flutter Engine from 703c9a14ac7f to 8cc6d6d5efdb (1 revision) (flutter/flutter#129339) 2023-06-22 [email protected] Roll Flutter Engine from d9530e2b87de to 703c9a14ac7f (1 revision) (flutter/flutter#129337) 2023-06-22 [email protected] Roll Flutter Engine from c6251a69a09a to d9530e2b87de (1 revision) (flutter/flutter#129334) 2023-06-22 [email protected] Roll Flutter Engine from 08aaa88bf67f to c6251a69a09a (10 revisions) (flutter/flutter#129331) 2023-06-22 [email protected] Manual roll of packages to 9af50d4 (flutter/flutter#129328) 2023-06-21 [email protected] Roll Flutter Engine from 090fae83548a to 08aaa88bf67f (3 revisions) (flutter/flutter#129306) 2023-06-21 [email protected] [framework,web] add FlutterTimeline and semantics benchmarks that use it (flutter/flutter#128366) 2023-06-21 [email protected] Roll pub packages (flutter/flutter#128966) 2023-06-21 [email protected] Remove incorrect non-nullable assumption from `ShapeDecoration.lerp` (flutter/flutter#129298) 2023-06-21 [email protected] Gracefully handle negative position in getWordAtOffset (flutter/flutter#128464) 2023-06-21 [email protected] [flutter_tools] add a gradle error handler for could not open cache directory (flutter/flutter#129222) 2023-06-21 [email protected] Roll Flutter Engine from f973fb4636d3 to 090fae83548a (5 revisions) (flutter/flutter#129293) 2023-06-21 [email protected] Selection area right click behavior should match native (flutter/flutter#128224) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
3 participants
I received reports of crashes due to a negative position in getWordAtOffset, and this prevents them. It looks like the existing logic was meant to handle negative positions anyway, but the terms were reversed?
I was not able to actually reproduce the crash. I can't figure out a way for
getPositionForOffsetto returnoffset: 0, affinity: upstream.In order to test this without a real repro, I made getWordAtOffset
@visibleForTesting. Is there any other way to test it?