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

Commit 21b456d

Browse files
authored
Check for index bounds in RTL handling for trailing whitespace runs. (#12336)
* Check for special case index out of bounds condition for leading space * Add TODO * Rework to pass tests * More robust check for leading * Minor adjustment * Fix order bug * Do not modify for leading space * Fix test value * Condition
1 parent 4ea0cf3 commit 21b456d

File tree

2 files changed

+65
-2
lines changed

2 files changed

+65
-2
lines changed

third_party/txt/src/txt/paragraph_txt.cc

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,9 @@ bool ParagraphTxt::ComputeBidiRuns(std::vector<BidiRun>* result) {
415415
//
416416
// This only applies to the final whitespace at the end as other whitespace is
417417
// no longer ambiguous when surrounded by additional text.
418+
419+
// TODO(garyq): Handle this in the text editor caret code instead at layout
420+
// level.
418421
bool has_trailing_whitespace = false;
419422
int32_t bidi_run_start, bidi_run_length;
420423
if (bidi_run_count > 1) {
@@ -427,8 +430,17 @@ bool ParagraphTxt::ComputeBidiRuns(std::vector<BidiRun>* result) {
427430
U16_GET(text_.data(), 0, bidi_run_start + bidi_run_length - 1,
428431
static_cast<int>(text_.size()), last_char);
429432
if (u_hasBinaryProperty(last_char, UCHAR_WHITE_SPACE)) {
430-
has_trailing_whitespace = true;
431-
bidi_run_count--;
433+
// Check if the trailing whitespace occurs before the previous run or
434+
// not. If so, this trailing whitespace was a leading whitespace.
435+
int32_t second_last_bidi_run_start, second_last_bidi_run_length;
436+
ubidi_getVisualRun(bidi.get(), bidi_run_count - 2,
437+
&second_last_bidi_run_start,
438+
&second_last_bidi_run_length);
439+
if (bidi_run_start ==
440+
second_last_bidi_run_start + second_last_bidi_run_length) {
441+
has_trailing_whitespace = true;
442+
bidi_run_count--;
443+
}
432444
}
433445
}
434446
}

third_party/txt/tests/paragraph_unittests.cc

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2327,6 +2327,57 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(JustifyRTLNewLine)) {
23272327
}
23282328
}
23292329

2330+
TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(LeadingSpaceRTL)) {
2331+
const char* text = " leading space";
2332+
2333+
auto icu_text = icu::UnicodeString::fromUTF8(text);
2334+
std::u16string u16_text(icu_text.getBuffer(),
2335+
icu_text.getBuffer() + icu_text.length());
2336+
2337+
txt::ParagraphStyle paragraph_style;
2338+
paragraph_style.max_lines = 14;
2339+
paragraph_style.text_align = TextAlign::justify;
2340+
paragraph_style.text_direction = TextDirection::rtl;
2341+
txt::ParagraphBuilderTxt builder(paragraph_style, GetTestFontCollection());
2342+
2343+
txt::TextStyle text_style;
2344+
text_style.font_families = std::vector<std::string>(1, "Ahem");
2345+
text_style.font_size = 26;
2346+
text_style.color = SK_ColorBLACK;
2347+
text_style.height = 1;
2348+
builder.PushStyle(text_style);
2349+
2350+
builder.AddText(u16_text);
2351+
2352+
builder.Pop();
2353+
2354+
auto paragraph = BuildParagraph(builder);
2355+
size_t paragraph_width = GetTestCanvasWidth() - 100;
2356+
paragraph->Layout(paragraph_width);
2357+
2358+
paragraph->Paint(GetCanvas(), 0, 0);
2359+
2360+
SkPaint paint;
2361+
paint.setStyle(SkPaint::kStroke_Style);
2362+
paint.setAntiAlias(true);
2363+
paint.setStrokeWidth(1);
2364+
2365+
// Tests for GetRectsForRange()
2366+
Paragraph::RectHeightStyle rect_height_style =
2367+
Paragraph::RectHeightStyle::kMax;
2368+
Paragraph::RectWidthStyle rect_width_style =
2369+
Paragraph::RectWidthStyle::kTight;
2370+
paint.setColor(SK_ColorRED);
2371+
std::vector<txt::Paragraph::TextBox> boxes =
2372+
paragraph->GetRectsForRange(0, 100, rect_height_style, rect_width_style);
2373+
for (size_t i = 0; i < boxes.size(); ++i) {
2374+
GetCanvas()->drawRect(boxes[i].rect, paint);
2375+
}
2376+
ASSERT_EQ(boxes.size(), 2ull);
2377+
2378+
// This test should crash if behavior regresses.
2379+
}
2380+
23302381
TEST_F(ParagraphTest, DecorationsParagraph) {
23312382
txt::ParagraphStyle paragraph_style;
23322383
paragraph_style.max_lines = 14;

0 commit comments

Comments
 (0)