Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 592cbab

Browse files
robert-ancellrobert-ancell
authored
Fix right-to-left selection not working. (#18951)
Fixes flutter/flutter#57636
1 parent 3864e3f commit 592cbab

File tree

3 files changed

+132
-15
lines changed

3 files changed

+132
-15
lines changed

‎shell/platform/common/cpp/text_input_model.cc‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,7 @@ TextInputModel::~TextInputModel() = default;
3737
bool TextInputModel::SetEditingState(size_t selection_base,
3838
size_t selection_extent,
3939
const std::string& text) {
40-
if (selection_base > selection_extent) {
41-
return false;
42-
}
43-
// Only checks extent since it is implicitly greater-than-or-equal-to base.
44-
if (selection_extent > text.size()) {
40+
if (selection_base > text.size() || selection_extent > text.size()) {
4541
return false;
4642
}
4743
std::wstring_convert<std::codecvt_utf8_utf16<char16_t>, char16_t>
@@ -53,8 +49,7 @@ bool TextInputModel::SetEditingState(size_t selection_base,
5349
}
5450

5551
void TextInputModel::DeleteSelected() {
56-
selection_base_ = text_.erase(selection_base_, selection_extent_);
57-
// Moves extent back to base, so that it is a single cursor placement again.
52+
selection_base_ = text_.erase(selection_start(), selection_end());
5853
selection_extent_ = selection_base_;
5954
}
6055

@@ -178,7 +173,8 @@ bool TextInputModel::MoveCursorToEnd() {
178173
bool TextInputModel::MoveCursorForward() {
179174
// If about to move set to the end of the highlight (when not selecting).
180175
if (selection_base_ != selection_extent_) {
181-
selection_base_ = selection_extent_;
176+
selection_base_ = selection_end();
177+
selection_extent_ = selection_base_;
182178
return true;
183179
}
184180
// If not at the end, move the extent forward.
@@ -195,6 +191,7 @@ bool TextInputModel::MoveCursorBack() {
195191
// If about to move set to the beginning of the highlight
196192
// (when not selecting).
197193
if (selection_base_ != selection_extent_) {
194+
selection_base_ = selection_start();
198195
selection_extent_ = selection_base_;
199196
return true;
200197
}

‎shell/platform/common/cpp/text_input_model.h‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,14 @@ class TextInputModel {
7373

7474
// Attempts to move the cursor backward.
7575
//
76-
// Returns true if the cursor could be moved. Changes base and extent to be
77-
// equal to either the extent (if extent is at the end of the string), or
78-
// for extent to be equal to
76+
// Returns true if the cursor could be moved. If a selection is active, moves
77+
// to the start of the selection.
7978
bool MoveCursorBack();
8079

8180
// Attempts to move the cursor forward.
8281
//
83-
// Returns true if the cursor could be moved.
82+
// Returns true if the cursor could be moved. If a selection is active, moves
83+
// to the end of the selection.
8484
bool MoveCursorForward();
8585

8686
// Attempts to move the cursor to the beginning.
@@ -100,12 +100,12 @@ class TextInputModel {
100100
// GetText().
101101
int GetCursorOffset() const;
102102

103-
// The position of the cursor
103+
// The position where the selection starts.
104104
int selection_base() const {
105105
return static_cast<int>(selection_base_ - text_.begin());
106106
}
107107

108-
// The end of the selection
108+
// The position of the cursor.
109109
int selection_extent() const {
110110
return static_cast<int>(selection_extent_ - text_.begin());
111111
}
@@ -116,6 +116,18 @@ class TextInputModel {
116116
std::u16string text_;
117117
std::u16string::iterator selection_base_;
118118
std::u16string::iterator selection_extent_;
119+
120+
// Returns the left hand side of the selection.
121+
std::u16string::iterator selection_start() {
122+
return selection_base_ < selection_extent_ ? selection_base_
123+
: selection_extent_;
124+
}
125+
126+
// Returns the right hand side of the selection.
127+
std::u16string::iterator selection_end() {
128+
return selection_base_ > selection_extent_ ? selection_base_
129+
: selection_extent_;
130+
}
119131
};
120132

121133
} // namespace flutter

‎shell/platform/common/cpp/text_input_model_unittests.cc‎

Lines changed: 109 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,10 @@ TEST(TextInputModel, SetEditingStateSelection) {
4646

4747
TEST(TextInputModel, SetEditingStateReverseSelection) {
4848
auto model = std::make_unique<TextInputModel>();
49-
EXPECT_FALSE(model->SetEditingState(3, 1, "ABCDE"));
49+
EXPECT_TRUE(model->SetEditingState(4, 1, "ABCDE"));
50+
EXPECT_EQ(model->selection_base(), 4);
51+
EXPECT_EQ(model->selection_extent(), 1);
52+
EXPECT_STREQ(model->GetText().c_str(), "ABCDE");
5053
}
5154

5255
TEST(TextInputModel, SetEditingStateOutsideString) {
@@ -85,6 +88,15 @@ TEST(TextInputModel, AddCodePointSelection) {
8588
EXPECT_STREQ(model->GetText().c_str(), "AxE");
8689
}
8790

91+
TEST(TextInputModel, AddCodePointReverseSelection) {
92+
auto model = std::make_unique<TextInputModel>();
93+
EXPECT_TRUE(model->SetEditingState(4, 1, "ABCDE"));
94+
model->AddCodePoint('x');
95+
EXPECT_EQ(model->selection_base(), 2);
96+
EXPECT_EQ(model->selection_extent(), 2);
97+
EXPECT_STREQ(model->GetText().c_str(), "AxE");
98+
}
99+
88100
TEST(TextInputModel, AddCodePointSelectionWideCharacter) {
89101
auto model = std::make_unique<TextInputModel>();
90102
EXPECT_TRUE(model->SetEditingState(1, 4, "ABCDE"));
@@ -94,6 +106,15 @@ TEST(TextInputModel, AddCodePointSelectionWideCharacter) {
94106
EXPECT_STREQ(model->GetText().c_str(), "A😄E");
95107
}
96108

109+
TEST(TextInputModel, AddCodePointReverseSelectionWideCharacter) {
110+
auto model = std::make_unique<TextInputModel>();
111+
EXPECT_TRUE(model->SetEditingState(4, 1, "ABCDE"));
112+
model->AddCodePoint(0x1f604);
113+
EXPECT_EQ(model->selection_base(), 3);
114+
EXPECT_EQ(model->selection_extent(), 3);
115+
EXPECT_STREQ(model->GetText().c_str(), "A😄E");
116+
}
117+
97118
TEST(TextInputModel, AddText) {
98119
auto model = std::make_unique<TextInputModel>();
99120
model->AddText(u"ABCDE");
@@ -113,6 +134,15 @@ TEST(TextInputModel, AddTextSelection) {
113134
EXPECT_STREQ(model->GetText().c_str(), "AxyE");
114135
}
115136

137+
TEST(TextInputModel, AddTextReverseSelection) {
138+
auto model = std::make_unique<TextInputModel>();
139+
EXPECT_TRUE(model->SetEditingState(4, 1, "ABCDE"));
140+
model->AddText("xy");
141+
EXPECT_EQ(model->selection_base(), 3);
142+
EXPECT_EQ(model->selection_extent(), 3);
143+
EXPECT_STREQ(model->GetText().c_str(), "AxyE");
144+
}
145+
116146
TEST(TextInputModel, AddTextSelectionWideCharacter) {
117147
auto model = std::make_unique<TextInputModel>();
118148
EXPECT_TRUE(model->SetEditingState(1, 4, "ABCDE"));
@@ -122,6 +152,15 @@ TEST(TextInputModel, AddTextSelectionWideCharacter) {
122152
EXPECT_STREQ(model->GetText().c_str(), "A😄🙃E");
123153
}
124154

155+
TEST(TextInputModel, AddTextReverseSelectionWideCharacter) {
156+
auto model = std::make_unique<TextInputModel>();
157+
EXPECT_TRUE(model->SetEditingState(4, 1, "ABCDE"));
158+
model->AddText(u"😄🙃");
159+
EXPECT_EQ(model->selection_base(), 5);
160+
EXPECT_EQ(model->selection_extent(), 5);
161+
EXPECT_STREQ(model->GetText().c_str(), "A😄🙃E");
162+
}
163+
125164
TEST(TextInputModel, DeleteStart) {
126165
auto model = std::make_unique<TextInputModel>();
127166
EXPECT_TRUE(model->SetEditingState(0, 0, "ABCDE"));
@@ -167,6 +206,15 @@ TEST(TextInputModel, DeleteSelection) {
167206
EXPECT_STREQ(model->GetText().c_str(), "AE");
168207
}
169208

209+
TEST(TextInputModel, DeleteReverseSelection) {
210+
auto model = std::make_unique<TextInputModel>();
211+
EXPECT_TRUE(model->SetEditingState(4, 1, "ABCDE"));
212+
ASSERT_TRUE(model->Delete());
213+
EXPECT_EQ(model->selection_base(), 1);
214+
EXPECT_EQ(model->selection_extent(), 1);
215+
EXPECT_STREQ(model->GetText().c_str(), "AE");
216+
}
217+
170218
TEST(TextInputModel, DeleteSurroundingAtCursor) {
171219
auto model = std::make_unique<TextInputModel>();
172220
model->SetEditingState(2, 2, "ABCDE");
@@ -257,6 +305,15 @@ TEST(TextInputModel, DeleteSurroundingSelection) {
257305
EXPECT_STREQ(model->GetText().c_str(), "ABCE");
258306
}
259307

308+
TEST(TextInputModel, DeleteSurroundingReverseSelection) {
309+
auto model = std::make_unique<TextInputModel>();
310+
model->SetEditingState(4, 3, "ABCDE");
311+
EXPECT_TRUE(model->DeleteSurrounding(0, 1));
312+
EXPECT_EQ(model->selection_base(), 3);
313+
EXPECT_EQ(model->selection_extent(), 3);
314+
EXPECT_STREQ(model->GetText().c_str(), "ABCE");
315+
}
316+
260317
TEST(TextInputModel, BackspaceStart) {
261318
auto model = std::make_unique<TextInputModel>();
262319
EXPECT_TRUE(model->SetEditingState(0, 0, "ABCDE"));
@@ -302,6 +359,15 @@ TEST(TextInputModel, BackspaceSelection) {
302359
EXPECT_STREQ(model->GetText().c_str(), "AE");
303360
}
304361

362+
TEST(TextInputModel, BackspaceReverseSelection) {
363+
auto model = std::make_unique<TextInputModel>();
364+
EXPECT_TRUE(model->SetEditingState(4, 1, "ABCDE"));
365+
ASSERT_TRUE(model->Delete());
366+
EXPECT_EQ(model->selection_base(), 1);
367+
EXPECT_EQ(model->selection_extent(), 1);
368+
EXPECT_STREQ(model->GetText().c_str(), "AE");
369+
}
370+
305371
TEST(TextInputModel, MoveCursorForwardStart) {
306372
auto model = std::make_unique<TextInputModel>();
307373
EXPECT_TRUE(model->SetEditingState(0, 0, "ABCDE"));
@@ -347,6 +413,15 @@ TEST(TextInputModel, MoveCursorForwardSelection) {
347413
EXPECT_STREQ(model->GetText().c_str(), "ABCDE");
348414
}
349415

416+
TEST(TextInputModel, MoveCursorForwardReverseSelection) {
417+
auto model = std::make_unique<TextInputModel>();
418+
EXPECT_TRUE(model->SetEditingState(4, 1, "ABCDE"));
419+
EXPECT_TRUE(model->MoveCursorForward());
420+
EXPECT_EQ(model->selection_base(), 4);
421+
EXPECT_EQ(model->selection_extent(), 4);
422+
EXPECT_STREQ(model->GetText().c_str(), "ABCDE");
423+
}
424+
350425
TEST(TextInputModel, MoveCursorBackStart) {
351426
auto model = std::make_unique<TextInputModel>();
352427
EXPECT_TRUE(model->SetEditingState(0, 0, "ABCDE"));
@@ -392,6 +467,15 @@ TEST(TextInputModel, MoveCursorBackSelection) {
392467
EXPECT_STREQ(model->GetText().c_str(), "ABCDE");
393468
}
394469

470+
TEST(TextInputModel, MoveCursorBackReverseSelection) {
471+
auto model = std::make_unique<TextInputModel>();
472+
model->SetEditingState(4, 1, "ABCDE");
473+
EXPECT_TRUE(model->MoveCursorBack());
474+
EXPECT_EQ(model->selection_base(), 1);
475+
EXPECT_EQ(model->selection_extent(), 1);
476+
EXPECT_STREQ(model->GetText().c_str(), "ABCDE");
477+
}
478+
395479
TEST(TextInputModel, MoveCursorToBeginningStart) {
396480
auto model = std::make_unique<TextInputModel>();
397481
model->SetEditingState(0, 0, "ABCDE");
@@ -428,6 +512,15 @@ TEST(TextInputModel, MoveCursorToBeginningSelection) {
428512
EXPECT_STREQ(model->GetText().c_str(), "ABCDE");
429513
}
430514

515+
TEST(TextInputModel, MoveCursorToBeginningReverseSelection) {
516+
auto model = std::make_unique<TextInputModel>();
517+
model->SetEditingState(4, 1, "ABCDE");
518+
EXPECT_TRUE(model->MoveCursorToBeginning());
519+
EXPECT_EQ(model->selection_base(), 0);
520+
EXPECT_EQ(model->selection_extent(), 0);
521+
EXPECT_STREQ(model->GetText().c_str(), "ABCDE");
522+
}
523+
431524
TEST(TextInputModel, MoveCursorToEndStart) {
432525
auto model = std::make_unique<TextInputModel>();
433526
model->SetEditingState(0, 0, "ABCDE");
@@ -464,6 +557,15 @@ TEST(TextInputModel, MoveCursorToEndSelection) {
464557
EXPECT_STREQ(model->GetText().c_str(), "ABCDE");
465558
}
466559

560+
TEST(TextInputModel, MoveCursorToEndReverseSelection) {
561+
auto model = std::make_unique<TextInputModel>();
562+
model->SetEditingState(4, 1, "ABCDE");
563+
EXPECT_TRUE(model->MoveCursorToEnd());
564+
EXPECT_EQ(model->selection_base(), 5);
565+
EXPECT_EQ(model->selection_extent(), 5);
566+
EXPECT_STREQ(model->GetText().c_str(), "ABCDE");
567+
}
568+
467569
TEST(TextInputModel, GetCursorOffset) {
468570
auto model = std::make_unique<TextInputModel>();
469571
// These characters take 1, 2, 3 and 4 bytes in UTF-8.
@@ -485,4 +587,10 @@ TEST(TextInputModel, GetCursorOffsetSelection) {
485587
EXPECT_EQ(model->GetCursorOffset(), 4);
486588
}
487589

590+
TEST(TextInputModel, GetCursorOffsetReverseSelection) {
591+
auto model = std::make_unique<TextInputModel>();
592+
model->SetEditingState(4, 1, "ABCDE");
593+
EXPECT_EQ(model->GetCursorOffset(), 1);
594+
}
595+
488596
} // namespace flutter

0 commit comments

Comments
 (0)