Bug 466814 - dolphin rename file selects part of the extension when name contains composed glyphs (diacritics, emojis)
Summary: dolphin rename file selects part of the extension when name contains composed...
Status: RESOLVED FIXED
Alias: None
Product: dolphin
Classification: Applications
Component: view-engine: general (show other bugs)
Version: 22.08.2
Platform: Other Other
: NOR normal
Target Milestone: ---
Assignee: Dolphin Bug Assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-03-04 07:52 UTC by kdebugs
Modified: 2024-08-13 17:39 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description kdebugs 2023-03-04 07:52:09 UTC
SUMMARY
Dolphin improperly counts characters when attempting to select everything but the ".ext".

STEPS TO REPRODUCE
1. Make a file named "épée.txt"
[NB: each é is composed of U+0065 (lowercase letter e) plus U+0301 (combining acute accent).  The filename does NOT use the precomposed character U+00E9.]
2. Select the file and hit F2 (or right-click and select Rename...)

OBSERVED RESULT
Dolphin selects "épée.t"

EXPECTED RESULT
Dolphin should select "épée"

SOFTWARE/OS VERSIONS
Kubuntu 22.10
KDE Plasma Version: 5.25.5
KDE Frameworks Version: 5.98.0
Qt Version: 5.15.6

ADDITIONAL INFORMATION

Second example, but with emojis:
"zz❤️❤️.txt" (NB: each of the two hearts is composed of U+2764 followed by U+FE0F)

Without looking at the code, I'm guessing someone did something naive like QTextCursor.movePosition() by a certain number of characters.  The programmer should understanding that Qt counts cursor positions in QChar (always 16-bit, so sometimes partial characters), however movePosition jumps over composed/joined characters as a unit, so neither setPosition() nor movePosition() are suitable for placing the cursor at a given character offset.  You must first encode the string you want to select as utf-16 and divide the resulting length in bytes by 2.  This number is what you use to determine a QChar offset which can be passed to setPosition.

[If done right, that should put you in front of the period (.) which is safe enough.  In other contexts, you would want to also make sure that your moving by x number of characters doesn't land you in the middle of a composed glyph.  You can do that by using movePosition to advance 1 character and then again to go back 1 character.  If you weren't in the middle of a glyph, this doesn't move you, but if you were, it safely puts you in front of whatever you were in.]
Comment 1 kde 2023-05-20 09:47:44 UTC
I also just experienced this bug. Dolphin version 23.04.0.

The amount of open bugs around dolphin just shocked me!
Comment 2 Bug Janitor Service 2024-08-09 03:24:57 UTC
A possibly relevant merge request was started @ https://invent.kde.org/system/dolphin/-/merge_requests/809
Comment 3 fanzhuyifan 2024-08-13 16:14:41 UTC
Git commit 021365dceb590a14bfcdb904ea05ffdd69b7d663 by Yifan Zhu.
Committed on 13/08/2024 at 16:09.
Pushed by fanzhuyifan into branch 'master'.

KStandardItemListWidget: select by number of unicode chars

Previously during rename, the number of QChar is used for selection,
which might be different from number of unicode characters.

Test plan:
- create the file zz❤️❤️.txt
- rename the file
- verify that the first 4 characters are correctly selected, which
  didn't work before the patch.

M  +8    -9    src/kitemviews/kfileitemlistwidget.cpp
M  +1    -1    src/kitemviews/kfileitemlistwidget.h
M  +12   -1    src/kitemviews/kstandarditemlistwidget.cpp
M  +3    -1    src/kitemviews/kstandarditemlistwidget.h

https://invent.kde.org/system/dolphin/-/commit/021365dceb590a14bfcdb904ea05ffdd69b7d663
Comment 4 fanzhuyifan 2024-08-13 17:39:02 UTC
Git commit 34d3f770bcc5759f46b0ccc8fcad9956c7fbed0a by Yifan Zhu.
Committed on 13/08/2024 at 16:29.
Pushed by meven into branch 'release/24.08'.

KStandardItemListWidget: select by number of unicode chars

Previously during rename, the number of QChar is used for selection,
which might be different from number of unicode characters.

Test plan:
- create the file zz❤️❤️.txt
- rename the file
- verify that the first 4 characters are correctly selected, which
  didn't work before the patch.


(cherry picked from commit 021365dceb590a14bfcdb904ea05ffdd69b7d663)

Co-authored-by: Yifan Zhu <fanzhuyifan@gmail.com>

M  +8    -9    src/kitemviews/kfileitemlistwidget.cpp
M  +1    -1    src/kitemviews/kfileitemlistwidget.h
M  +12   -1    src/kitemviews/kstandarditemlistwidget.cpp
M  +3    -1    src/kitemviews/kstandarditemlistwidget.h

https://invent.kde.org/system/dolphin/-/commit/34d3f770bcc5759f46b0ccc8fcad9956c7fbed0a