Commit f7ed1fa9 authored by Oriol Brufau's avatar Oriol Brufau Committed by Commit Bot

Avoid DCHECK failure when reusing space and newline with white-space:pre

Consider this code:
  <style>i { white-space: pre-line }</style>
  X<i> </i><i>&#10;</i>

The first time that NGInlineNode::CollectInlines runs, the text data
will be "X", then "X ", and then AppendForcedBreakCollapseWhitespace
will remove the trailing whitespace before inserting the forced break,
so we get "X\n".

But if some text is appended dynamically, CollectInlines will run again,
and this time it will try to reuse existing items in AppendTextReusing.
Before this patch, this would produce "X \n", which would trigger a
DCHECK failure in ComputeOffsetMapping because it's a different string.

Therefore, this patch prevents AppendTextReusing from reusing existing
items if the last item to collapse with ends with collapsible spaces and
the first item to reuse has 'white-space:pre' and begins with a newline.

Bug: 1136688

TEST=external/wpt/css/css-text/white-space/pre-line-with-space-and-newline.html

Change-Id: I0ff71b125b66256ec3b5c263605c3c116f9ac55f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2466279Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/master@{#816641}
parent c98ef544
......@@ -294,14 +294,23 @@ bool NGInlineItemsBuilderTemplate<OffsetMappingBuilder>::AppendTextReusing(
// TODO(layout-dev): Handle cases where the old items are not consecutive.
const ComputedStyle& new_style = layout_text->StyleRef();
bool collapse_spaces = new_style.CollapseWhiteSpace();
bool preserve_newlines = new_style.PreserveNewline();
if (NGInlineItem* last_item = LastItemToCollapseWith(items_)) {
if (collapse_spaces) {
switch (last_item->EndCollapseType()) {
case NGInlineItem::kCollapsible:
// If the original string starts with a collapsible space, it may be
// collapsed.
if (original_string[old_item0.StartOffset()] == kSpaceCharacter)
return false;
switch (original_string[old_item0.StartOffset()]) {
case kSpaceCharacter:
// If the original string starts with a collapsible space, it may
// be collapsed.
return false;
case kNewlineCharacter:
// Collapsible spaces immediately before a preserved newline
// should be removed to be consistent with
// AppendForcedBreakCollapseWhitespace.
if (preserve_newlines)
return false;
}
// If the last item ended with a collapsible space run with segment
// breaks, we need to run the full algorithm to apply segment break
// rules. This may result in removal of the space in the last item.
......@@ -363,7 +372,7 @@ bool NGInlineItemsBuilderTemplate<OffsetMappingBuilder>::AppendTextReusing(
return false;
}
if (new_style.PreserveNewline()) {
if (preserve_newlines) {
// We exit and then re-enter all bidi contexts around a forced break. So, We
// must go through the full pipeline to ensure that we exit and enter the
// correct bidi contexts the re-layout.
......
......@@ -202,6 +202,7 @@ crbug.com/591099 external/wpt/css/css-text/text-transform/text-transform-shaping
crbug.com/591099 external/wpt/css/css-text/white-space/control-chars-00C.html [ Failure ]
crbug.com/591099 external/wpt/css/css-text/white-space/line-edge-white-space-collapse-001.html [ Failure ]
crbug.com/591099 external/wpt/css/css-text/white-space/line-edge-white-space-collapse-002.html [ Failure ]
crbug.com/591099 external/wpt/css/css-text/white-space/pre-line-with-space-and-newline.html [ Failure ]
crbug.com/591099 external/wpt/css/css-text/white-space/white-space-pre-wrap-trailing-spaces-012.html [ Failure ]
crbug.com/591099 external/wpt/css/css-text/white-space/white-space-pre-wrap-trailing-spaces-013.html [ Failure ]
crbug.com/591099 external/wpt/css/css-text/white-space/white-space-pre-wrap-trailing-spaces-014.html [ Failure ]
......
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Text Test: 'white-space: pre-line' with space and newline</title>
<link rel="author" title="Oriol Brufau" href="mailto:obrufau@igalia.com">
<link rel="help" href="https://drafts.csswg.org/css-text/#white-space-phase-1">
<link rel="help" href="https://crbug.com/1136688">
<link rel="match" href="../../reference/ref-filled-green-200px-square.html">
<meta name="assert" content="
Checks that collapsible spaces immediately preceding a sequent break are removed.
That still applies if they are separated into different inline elements.
Also, if some text is inserted dynamically, the browser should not crash.">
<link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
<style>
span {
font: 25px/1 Ahem;
background: red;
color: green;
}
i {
white-space: pre-line;
}
</style>
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div class="static together">
<span>XXXXXXXX<i> &#10;</i>XXXXXXXX</span>
</div>
<div class="static separated">
<span>XXXXXXXX<i> </i><i>&#10;</i>XXXXXXXX</span>
</div>
<div class="dynamic together">
<span>XXXXXXXX<i> &#10;</i></span>
</div>
<div class="dynamic separated">
<span>XXXXXXXX<i> </i><i>&#10;</i></span>
</div>
<script>
// Force layout
document.body.offsetLeft;
// Insert text, should not crash
for (let span of document.querySelectorAll(".dynamic > span")) {
span.append("XXXXXXXX");
}
</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