Prevent hyphenation break 'offset' to be equal to 'start'

When looking for the previous/next breaking opportunity, we might need to evaluate hyphenation options, if enabled. The hyphenation logic expects the words without trailing spaces. However, since our change in r807457 we only consider breaking opportunities after spaces. We have adapted the code in the ShapingLineBreaker that was intended to evaluate the hyphenation opportunities and prepare the words to be hyphenated. Basically, we decided to use the non-hangable-run-end when required. However, this non-hangable-run-end position might be equal or smaller than the 'start' position we are considering. This might lead to a an invalid result when invoking CheckBreakOffset. We also have another DCHECK that might be violated in this scenario; it's ensuring that if we are under an overflow case, either we are processing a 'break-spaces' CSS value or the 'candidate' is not a breakable space. In any case, we mist ensure that the 'word_end', when used as the non-hangable-run-end, is always greater than the 'start' value. Bug: 1133254 Change-Id: I085102c6c0576ba26e1d398f51a0fea72a57e599 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2446690 Commit-Queue: Koji Ishii <kojii@chromium.org> Reviewed-by: Koji Ishii <kojii@chromium.org> Cr-Commit-Position: refs/heads/master@{#813583}

Prevent hyphenation break 'offset' to be equal to 'start'
When looking for the previous/next breaking opportunity, we might need to evaluate hyphenation options, if enabled. The hyphenation logic expects the words without trailing spaces. However, since our change in r807457 we only consider breaking opportunities after spaces. We have adapted the code in the ShapingLineBreaker that was intended to evaluate the hyphenation opportunities and prepare the words to be hyphenated. Basically, we decided to use the non-hangable-run-end when required. However, this non-hangable-run-end position might be equal or smaller than the 'start' position we are considering. This might lead to a an invalid result when invoking CheckBreakOffset. We also have another DCHECK that might be violated in this scenario; it's ensuring that if we are under an overflow case, either we are processing a 'break-spaces' CSS value or the 'candidate' is not a breakable space. In any case, we mist ensure that the 'word_end', when used as the non-hangable-run-end, is always greater than the 'start' value. Bug: 1133254 Change-Id: I085102c6c0576ba26e1d398f51a0fea72a57e599 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2446690 Commit-Queue: Koji Ishii <kojii@chromium.org> Reviewed-by: Koji Ishii <kojii@chromium.org> Cr-Commit-Position: refs/heads/master@{#813583}
dfbe5b22 · Javier Fernández García-Boente · Commit Bot · 03efecec · dfbe5b22 · dfbe5b22
Commit dfbe5b22 authored Oct 04, 2020 by Javier Fernández García-Boente Committed by Commit Bot Oct 04, 2020
2 changed files
--- a/third_party/blink/renderer/platform/fonts/shaping/shaping_line_breaker.cc
+++ b/third_party/blink/renderer/platform/fonts/shaping/shaping_line_breaker.cc
@@ -124,7 +124,7 @@ ShapingLineBreaker::BreakOpportunity ShapingLineBreaker::Hyphenate(
  const String& text = GetText();
  unsigned word_end = break_iterator_->NextBreakOpportunity(offset);
  if (word_end != offset && IsBreakableSpace(text[word_end - 1]))
-    word_end = std::max(start, FindNonHangableEnd(text, word_end - 1));
+    word_end = std::max(start + 1, FindNonHangableEnd(text, word_end - 1));
  if (word_end == offset) {
    DCHECK(IsBreakableSpace(text[offset]) ||
           offset == break_iterator_->PreviousBreakOpportunity(offset, start));

--- a/third_party/blink/web_tests/external/wpt/css/css-text/hyphens/hyphens-auto-and-contenteditable-crash.html
+++ b/third_party/blink/web_tests/external/wpt/css/css-text/hyphens/hyphens-auto-and-contenteditable-crash.html
+<!doctype html>
+<title>CSS Text Test: Hyphenation in a contenteditable with a x20+x9+x20 trailing space sequence, makes chrome to crash</title>
+<link rel="help" href="https://crbug.com/1133254">
+<style>
+    span {
+        display: table-row;
+        hyphens: auto;
+    }
+</style>
+<div contenteditable=plaintext-only>
+    <span>XXX	 </span>
+</div>