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
This commit is contained in:
Gary Qian 2019-09-24 04:21:35 +08:00 committed by GitHub
parent 4ea0cf3a1a
commit 21b456d52b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 65 additions and 2 deletions

View File

@ -415,6 +415,9 @@ bool ParagraphTxt::ComputeBidiRuns(std::vector<BidiRun>* result) {
//
// This only applies to the final whitespace at the end as other whitespace is
// no longer ambiguous when surrounded by additional text.
// TODO(garyq): Handle this in the text editor caret code instead at layout
// level.
bool has_trailing_whitespace = false;
int32_t bidi_run_start, bidi_run_length;
if (bidi_run_count > 1) {
@ -427,8 +430,17 @@ bool ParagraphTxt::ComputeBidiRuns(std::vector<BidiRun>* result) {
U16_GET(text_.data(), 0, bidi_run_start + bidi_run_length - 1,
static_cast<int>(text_.size()), last_char);
if (u_hasBinaryProperty(last_char, UCHAR_WHITE_SPACE)) {
has_trailing_whitespace = true;
bidi_run_count--;
// Check if the trailing whitespace occurs before the previous run or
// not. If so, this trailing whitespace was a leading whitespace.
int32_t second_last_bidi_run_start, second_last_bidi_run_length;
ubidi_getVisualRun(bidi.get(), bidi_run_count - 2,
&second_last_bidi_run_start,
&second_last_bidi_run_length);
if (bidi_run_start ==
second_last_bidi_run_start + second_last_bidi_run_length) {
has_trailing_whitespace = true;
bidi_run_count--;
}
}
}
}

View File

@ -2327,6 +2327,57 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(JustifyRTLNewLine)) {
}
}
TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(LeadingSpaceRTL)) {
const char* text = " leading space";
auto icu_text = icu::UnicodeString::fromUTF8(text);
std::u16string u16_text(icu_text.getBuffer(),
icu_text.getBuffer() + icu_text.length());
txt::ParagraphStyle paragraph_style;
paragraph_style.max_lines = 14;
paragraph_style.text_align = TextAlign::justify;
paragraph_style.text_direction = TextDirection::rtl;
txt::ParagraphBuilderTxt builder(paragraph_style, GetTestFontCollection());
txt::TextStyle text_style;
text_style.font_families = std::vector<std::string>(1, "Ahem");
text_style.font_size = 26;
text_style.color = SK_ColorBLACK;
text_style.height = 1;
builder.PushStyle(text_style);
builder.AddText(u16_text);
builder.Pop();
auto paragraph = BuildParagraph(builder);
size_t paragraph_width = GetTestCanvasWidth() - 100;
paragraph->Layout(paragraph_width);
paragraph->Paint(GetCanvas(), 0, 0);
SkPaint paint;
paint.setStyle(SkPaint::kStroke_Style);
paint.setAntiAlias(true);
paint.setStrokeWidth(1);
// Tests for GetRectsForRange()
Paragraph::RectHeightStyle rect_height_style =
Paragraph::RectHeightStyle::kMax;
Paragraph::RectWidthStyle rect_width_style =
Paragraph::RectWidthStyle::kTight;
paint.setColor(SK_ColorRED);
std::vector<txt::Paragraph::TextBox> boxes =
paragraph->GetRectsForRange(0, 100, rect_height_style, rect_width_style);
for (size_t i = 0; i < boxes.size(); ++i) {
GetCanvas()->drawRect(boxes[i].rect, paint);
}
ASSERT_EQ(boxes.size(), 2ull);
// This test should crash if behavior regresses.
}
TEST_F(ParagraphTest, DecorationsParagraph) {
txt::ParagraphStyle paragraph_style;
paragraph_style.max_lines = 14;