Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect caret position with complex scripts #118403

Closed
tgucio opened this issue Jan 12, 2023 · 6 comments · Fixed by #122480
Closed

Incorrect caret position with complex scripts #118403

tgucio opened this issue Jan 12, 2023 · 6 comments · Fixed by #122480
Assignees
Labels
a: desktop Running on desktop a: internationalization Supporting other languages or locales. (aka i18n) a: text input Entering text in a text field or keyboard related problems f: material design flutter/packages/flutter/material repository. found in release: 3.3 Found to occur in 3.3 found in release: 3.7 Found to occur in 3.7 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list r: fixed Issue is closed as already fixed in a newer version

Comments

@tgucio
Copy link
Contributor

tgucio commented Jan 12, 2023

caret

Steps to Reproduce

  1. Execute flutter run on the code sample on a platform with HW keyboard
  2. Traverse the text using arrow left/right keys

Expected results: Caret advances correctly

Actual results: Caret jumps back and forth

Code sample
import 'package:flutter/material.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(primarySwatch: Colors.blue),
      home: const MyHomePage(title: 'Flutter Demo Home Page'),
    );
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({super.key, required this.title});

  final String title;

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  @override
  Widget build(BuildContext context) {
    const TextStyle style = TextStyle(fontSize: 16.0);
    return Scaffold(
      appBar: AppBar(title: Text(widget.title)),
      body: Center(
        child: Column(
          children: <Widget>[
            const SizedBox(height: 100.0),
            TextField(controller: TextEditingController(text: 'प्राप्त वर्णन प्रव्रुति')),
            const SizedBox(height: 100.0),
          ],
        ),
      ),
    );
  }
}
Logs
% flutter --version
Flutter 3.7.0-20.0.pre.6 • channel master • https://github.com/flutter/flutter.git
Framework • revision f8628b5cb1 (15 hours ago) • 2023-01-11 18:48:03 -0800
Engine • revision 4a8d6866a1
Tools • Dart 3.0.0 (build 3.0.0-112.0.dev) • DevTools 2.20.0
@huycozy huycozy added the in triage Presently being triaged by the triage team label Jan 13, 2023
@huycozy
Copy link
Member

huycozy commented Jan 13, 2023

Hi @tgucio, thanks for filing the issue. I also can reproduce this on the latest stable and master channels.

On the Web, it seems better even though the cursor doesn't move one step when moving to the space character between words.

This issue makes me remember the old issue that I have reproduced before: #106472. These issues seem to be similar since the it looks better on Web and they are all about cursor traversal (but they are different languages)

flutter doctor -v (stable and master)
[✓] Flutter (Channel stable, 3.3.10, on macOS 13.0.1 22A400 darwin-x64, locale en-VN)
    • Flutter version 3.3.10 on channel stable at /Users/huynq/Documents/GitHub/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 135454af32 (13 hours ago), 2022-12-15 07:36:55 -0800
    • Engine revision 3316dd8728
    • Dart version 2.18.6
    • DevTools version 2.15.0

[✓] Android toolchain - develop for Android devices (Android SDK version 31.0.0)
    • Android SDK at /Users/huynq/Library/Android/sdk
    • Platform android-33, build-tools 31.0.0
    • ANDROID_HOME = /Users/huynq/Library/Android/sdk
    • Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14B47b
    • CocoaPods version 1.11.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2021.3)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)

[✓] IntelliJ IDEA Community Edition (version 2022.1.1)
    • IntelliJ at /Users/huynq/Library/Application Support/JetBrains/Toolbox/apps/IDEA-C/ch-0/221.5591.52/IntelliJ IDEA CE.app
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart

[✓] VS Code (version 1.73.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.54.0

[✓] Connected device (3 available)
    • SM T225 (mobile) • R9JT3004VRJ • android-arm64  • Android 12 (API 31)
    • macOS (desktop)  • macos       • darwin-x64     • macOS 13.0.1 22A400 darwin-x64
    • Chrome (web)     • chrome      • web-javascript • Google Chrome 108.0.5359.124

[✓] HTTP Host Availability
    • All required HTTP hosts are available

• No issues found!
[!] Flutter (Channel master, 3.7.0-20.0.pre.26, on macOS 13.0.1 22A400 darwin-x64, locale en-VN)
    • Flutter version 3.7.0-20.0.pre.26 on channel master at /Users/huynq/Documents/GitHub/flutter_master
    ! Warning: `flutter` on your path resolves to /Users/huynq/Documents/GitHub/flutter/bin/flutter, which is not inside your current Flutter SDK checkout at /Users/huynq/Documents/GitHub/flutter_master. Consider adding /Users/huynq/Documents/GitHub/flutter_master/bin to the front of your path.
    ! Warning: `dart` on your path resolves to /Users/huynq/Documents/GitHub/flutter/bin/dart, which is not inside your current Flutter SDK checkout at /Users/huynq/Documents/GitHub/flutter_master. Consider adding /Users/huynq/Documents/GitHub/flutter_master/bin to the front of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision f7b444e117 (17 minutes ago), 2023-01-13 03:11:08 +0100
    • Engine revision 8aa26baa9d
    • Dart version 3.0.0 (build 3.0.0-119.0.dev)
    • DevTools version 2.20.0
    • If those were intentional, you can disregard the above warnings; however it is recommended to use "git" directly to perform update checks and upgrades.

[✓] Android toolchain - develop for Android devices (Android SDK version 31.0.0)
    • Android SDK at /Users/huynq/Library/Android/sdk
    • Platform android-33, build-tools 31.0.0
    • ANDROID_HOME = /Users/huynq/Library/Android/sdk
    • Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14B47b
    • CocoaPods version 1.11.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2021.3)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)

[✓] IntelliJ IDEA Community Edition (version 2022.1.1)
    • IntelliJ at /Users/huynq/Library/Application Support/JetBrains/Toolbox/apps/IDEA-C/ch-0/221.5591.52/IntelliJ IDEA CE.app
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart

[✓] VS Code (version 1.74.2)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.56.0

[✓] Connected device (2 available)
    • macOS (desktop) • macos  • darwin-x64     • macOS 13.0.1 22A400 darwin-x64
    • Chrome (web)    • chrome • web-javascript • Google Chrome 108.0.5359.124

[✓] HTTP Host Availability
    • All required HTTP hosts are available

! Doctor found issues in 1 category.

@huycozy huycozy added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. a: internationalization Supporting other languages or locales. (aka i18n) a: desktop Running on desktop has reproducible steps The issue has been confirmed reproducible and is ready to work on found in release: 3.3 Found to occur in 3.3 found in release: 3.7 Found to occur in 3.7 and removed in triage Presently being triaged by the triage team labels Jan 13, 2023
@tgucio
Copy link
Contributor Author

tgucio commented Jan 13, 2023

Thanks @huycozy. I think they're actually different issues. This one, I believe, has to do with incorrect calcs when text shaping is used (either standard rules for Indic scripts or GSUB, GPOS in fonts). #106472 has to do with incorrect word boundaries, seen with Han script which normally doesn't use text shaping.

@gspencergoog gspencergoog added the P2 Important issues not at the top of the work list label Jan 19, 2023
@gspencergoog
Copy link
Contributor

cc @jason-simmons @justinmc

@justinmc
Copy link
Contributor

Is this a duplicate of #118566? That issue may have a fix in progress in Skia.

@tgucio
Copy link
Contributor Author

tgucio commented Jan 19, 2023

@justinmc I don't think so: the Skia fix appears to be specific to RTL runs. Here it's LTR and my bet is that it has to do with what happens when the text shaper substitutes glyphs.

gnprice added a commit to gnprice/flutter that referenced this issue Mar 12, 2023
…rics

Fixes flutter#50563.
Fixes flutter#98516.
Fixes flutter#118403.
Fixes flutter#122477.

It turns out that `Paragraph.getBoxesForRange` will not accept a
negative index for the start of the range; if you pass one, it
will return an empty list.  We'd been doing that in
`TextPainter._getRectFromUpstream`; this was causing flutter#50563, flutter#98516,
and flutter#118403.  (Arguably this makes them duplicates of each other,
but they looked quite distinct until being debugged: they involve
complex emoji, zalgo text, and Indic script respectively.)

Fix that by clamping the start to zero, because what we really
intended here in that case is the whole text up to the given
end offset.

Then it turns out that we were sometimes reaching this line with a
negative end index too (when the selection is empty on reaching
`RenderEditable._updateSelectionExtentsVisibility`.)  That was
giving an empty list of boxes, but if we clamp the start but not
the end, we'd get back boxes for the whole text instead.  Avoid that
by short-circuiting that case at the top of the function, like we do
when the offset is out of bounds in the other direction.

Finally, when we get back more than one box from `getBoxesForRange`,
we were looking at the wrong end of the list: if we're looking
upstream, we want the end of the *last* preceding box, not the first
one.  Similarly when looking downstream we want the start of the
first following box, not the last one.  This was causing flutter#122477,
which I discovered while studying this code to diagnose flutter#50563.
Fix that too.
@huycozy huycozy added the r: fixed Issue is closed as already fixed in a newer version label Mar 28, 2023
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: desktop Running on desktop a: internationalization Supporting other languages or locales. (aka i18n) a: text input Entering text in a text field or keyboard related problems f: material design flutter/packages/flutter/material repository. found in release: 3.3 Found to occur in 3.3 found in release: 3.7 Found to occur in 3.7 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list r: fixed Issue is closed as already fixed in a newer version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants