Commit f0e301a3 authored by Javier Fernández García-Boente's avatar Javier Fernández García-Boente Committed by Commit Bot

Prevent break opportunity offset to surpass the item's range_end

When exploring breaking opportunities we were early return, and shaping
to the end, when we detected that current candidate overflow and next
breaking opportunity surpass the item's range end.

However, since we landed in r807457 the new TextBreakIterator's
behavior of  breaking always after the space run, we can get some
opportunities that surpass the item's range_end even if the candidate
doesn't overflow. An example of this could be when trailing spaces are
located in different items.

This patch ensures that even when the candidate doesn't overflow, we
never consider breaking opportunity offsets that surpass the item's
range end; since we are not overflowing, we'll proceed with the shaping
logic considering range_end as maximum of the breaking opportunity
offset.

Bug: 1128571
Change-Id: I704eb8091cdca00713fed15e22749bb633bacc71
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414196
Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808007}
parent 36e4acea
...@@ -308,7 +308,7 @@ scoped_refptr<const ShapeResultView> ShapingLineBreaker::ShapeLine( ...@@ -308,7 +308,7 @@ scoped_refptr<const ShapeResultView> ShapingLineBreaker::ShapeLine(
// We don't care whether this result contains only spaces if we // We don't care whether this result contains only spaces if we
// are breaking after any space. We shouldn't early return either // are breaking after any space. We shouldn't early return either
// in that case. // in that case.
if (!is_break_after_any_space && if (!is_break_after_any_space && break_opportunity.non_hangable_run_end &&
break_opportunity.non_hangable_run_end <= start) { break_opportunity.non_hangable_run_end <= start) {
// TODO (jfenandez): There may be cases where candidate_break is // TODO (jfenandez): There may be cases where candidate_break is
// not a breakable space but we also want to early return for // not a breakable space but we also want to early return for
...@@ -332,6 +332,7 @@ scoped_refptr<const ShapeResultView> ShapingLineBreaker::ShapeLine( ...@@ -332,6 +332,7 @@ scoped_refptr<const ShapeResultView> ShapingLineBreaker::ShapeLine(
result_out->non_hangable_run_end = break_opportunity.non_hangable_run_end; result_out->non_hangable_run_end = break_opportunity.non_hangable_run_end;
if (result_out->is_overflow) if (result_out->is_overflow)
return ShapeToEnd(start, first_safe, range_start, range_end); return ShapeToEnd(start, first_safe, range_start, range_end);
break_opportunity.offset = range_end;
} }
CheckBreakOffset(break_opportunity.offset, start, range_end); CheckBreakOffset(break_opportunity.offset, start, range_end);
...@@ -369,9 +370,9 @@ scoped_refptr<const ShapeResultView> ShapingLineBreaker::ShapeLine( ...@@ -369,9 +370,9 @@ scoped_refptr<const ShapeResultView> ShapingLineBreaker::ShapeLine(
// early return if offset >= range_end ? // early return if offset >= range_end ?
if (break_opportunity.offset == range_end) if (break_opportunity.offset == range_end)
reshape_line_end = false; reshape_line_end = false;
if (!is_break_after_any_space) { if (!is_break_after_any_space && break_opportunity.non_hangable_run_end) {
break_opportunity.offset = break_opportunity.offset =
std::max(start + 1, break_opportunity.non_hangable_run_end); std::max(start + 1, *break_opportunity.non_hangable_run_end);
} }
unsigned last_safe = break_opportunity.offset; unsigned last_safe = break_opportunity.offset;
if (reshape_line_end) { if (reshape_line_end) {
...@@ -381,9 +382,9 @@ scoped_refptr<const ShapeResultView> ShapingLineBreaker::ShapeLine( ...@@ -381,9 +382,9 @@ scoped_refptr<const ShapeResultView> ShapingLineBreaker::ShapeLine(
// preceding boundary is tried until the available space is sufficient. // preceding boundary is tried until the available space is sufficient.
while (true) { while (true) {
DCHECK_LE(start, break_opportunity.offset); DCHECK_LE(start, break_opportunity.offset);
if (!is_break_after_any_space) { if (!is_break_after_any_space && break_opportunity.non_hangable_run_end) {
break_opportunity.offset = break_opportunity.offset =
std::max(start + 1, break_opportunity.non_hangable_run_end); std::max(start + 1, *break_opportunity.non_hangable_run_end);
} }
last_safe = last_safe =
result_->CachedPreviousSafeToBreakOffset(break_opportunity.offset); result_->CachedPreviousSafeToBreakOffset(break_opportunity.offset);
......
...@@ -115,7 +115,6 @@ class PLATFORM_EXPORT ShapingLineBreaker final { ...@@ -115,7 +115,6 @@ class PLATFORM_EXPORT ShapingLineBreaker final {
public: public:
BreakOpportunity(unsigned new_offset, bool hyphenated) BreakOpportunity(unsigned new_offset, bool hyphenated)
: offset(new_offset), : offset(new_offset),
non_hangable_run_end(new_offset),
is_hyphenated(hyphenated) {} is_hyphenated(hyphenated) {}
BreakOpportunity(unsigned new_offset, unsigned run_end, bool hyphenated) BreakOpportunity(unsigned new_offset, unsigned run_end, bool hyphenated)
: offset(new_offset), : offset(new_offset),
...@@ -123,7 +122,7 @@ class PLATFORM_EXPORT ShapingLineBreaker final { ...@@ -123,7 +122,7 @@ class PLATFORM_EXPORT ShapingLineBreaker final {
is_hyphenated(hyphenated) {} is_hyphenated(hyphenated) {}
unsigned offset; unsigned offset;
unsigned non_hangable_run_end; base::Optional<unsigned> non_hangable_run_end;
bool is_hyphenated; bool is_hyphenated;
}; };
BreakOpportunity PreviousBreakOpportunity(unsigned offset, BreakOpportunity PreviousBreakOpportunity(unsigned offset,
......
<!DOCTYPE html>
<title>CSS Text Test: Altering the DOM generates empty text elements that cause line-breaking to crash</title>
<link rel="help" href="https://crbug.com/972992">
<style>
body {
width: 5pt;
hyphens: none;
}
</style>
<body onload=jsfuzzer()>
<table id="table1">x</table>
<content contenteditable="plaintext-only">
</body>
<script>
function jsfuzzer() {
document.all[0].appendChild(table1);
}
</script>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment