Commit a07580d3 authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot

[LayoutNG] Fix 3 cases of break opportunities after nowrap

This patch fixes lines to break in the following conditions:
1. When wrappable elements appear inside of nowrap elements.
2. When wrappable spaces after nowrap appear inside of nowrap
   elements.
3. When non-space break opportunities appear after nowrap.

fast/text/whitespace/018.html improves but still doesn't pass.
It doesn't pass in Edge/Gecko, and at least some of what it
expects look questionable. Further investigation is deferred
to future CLs.

Bug: 920177
Change-Id: Ieba4d446b818120f423b87a7f4a44b3c63a9d995
Reviewed-on: https://chromium-review.googlesource.com/c/1477629
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: default avatarXiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635180}
parent ac25a05e
......@@ -796,7 +796,7 @@ void NGLineBreaker::HandleTrailingSpaces(const NGInlineItem& item,
} else {
// Find the end of the run of space characters in this item.
// Other white space characters (e.g., tab) are not included in this item.
DCHECK(style.BreakOnlyAfterWhiteSpace());
DCHECK(!auto_wrap_ || style.BreakOnlyAfterWhiteSpace());
unsigned end = offset_;
while (end < item.EndOffset() && IsBreakableSpace(text[end]))
end++;
......@@ -1256,7 +1256,6 @@ bool NGLineBreaker::ComputeOpenTagResult(
void NGLineBreaker::HandleOpenTag(const NGInlineItem& item) {
NGInlineItemResult* item_result = AddItem(item);
DCHECK(!item_result->can_break_after);
if (ComputeOpenTagResult(item, constraint_space_, item_result)) {
position_ += item_result->inline_size;
......@@ -1269,10 +1268,16 @@ void NGLineBreaker::HandleOpenTag(const NGInlineItem& item) {
item_result->should_create_line_box = true;
}
bool was_auto_wrap = auto_wrap_;
DCHECK(item.Style());
const ComputedStyle& style = *item.Style();
SetCurrentStyle(style);
MoveToNextOf(item);
DCHECK(!item_result->can_break_after);
if (UNLIKELY(!was_auto_wrap && auto_wrap_ && item_results_->size() >= 2)) {
ComputeCanBreakAfter(std::prev(item_result));
}
}
void NGLineBreaker::HandleCloseTag(const NGInlineItem& item) {
......@@ -1292,32 +1297,30 @@ void NGLineBreaker::HandleCloseTag(const NGInlineItem& item) {
SetCurrentStyle(item.GetLayoutObject()->Parent()->StyleRef());
MoveToNextOf(item);
// Prohibit break before a close tag by setting can_break_after to the
// previous result.
// TODO(kojii): There should be a result before close tag, but there are cases
// that doesn't because of the way we handle trailing spaces. This needs to be
// revisited.
if (item_results_->size() >= 2) {
NGInlineItemResult* last = &(*item_results_)[item_results_->size() - 2];
if (was_auto_wrap == auto_wrap_) {
// If the line can break after the previous item, prohibit it and allow break
// after this close tag instead.
if (was_auto_wrap) {
if (item_results_->size() >= 2) {
NGInlineItemResult* last = std::prev(item_result);
item_result->can_break_after = last->can_break_after;
last->can_break_after = false;
return;
}
last->can_break_after = false;
if (!was_auto_wrap) {
DCHECK(auto_wrap_);
// When auto-wrap starts after no-wrap, the boundary is not allowed to
// wrap. However, when space characters follow the boundary, there should
// be a break opportunity after the space. The break_iterator cannot
// compute this because it considers break opportunities are before a run
// of spaces.
const String& text = Text();
if (offset_ < text.length() && IsBreakableSpace(text[offset_])) {
item_result->can_break_after = true;
return;
}
}
return;
}
DCHECK(!item_result->can_break_after);
if (!auto_wrap_)
return;
// When auto-wrap follows no-wrap, the boundary is determined by the break
// iterator. However, when space characters follow the boundary, the break
// iterator cannot compute this because it considers break opportunities are
// before a run of spaces.
const String& text = Text();
if (item_result->end_offset < text.length() &&
IsBreakableSpace(text[item_result->end_offset])) {
item_result->can_break_after = true;
return;
}
ComputeCanBreakAfter(item_result);
}
......
<!DOCTYPE html>
<meta charset="utf-8">
<style>
div {
width: 10ch;
border: 2px solid blue;
line-height: 1;
}
.ideo > div {
width: 1em;
}
</style>
<body>
<section>
<div>12345<br>67890</div>
<div>12345<br>67890</div>
<div>12345<br>67890</div>
<div>12345<br>67890</div>
<div>12345<br>67890</div>
<div>12345<br>67890</div>
</section>
<section class="ideo">
<div><br></div>
<div><br></div>
<div><br></div>
<div><br></div>
</section>
</body>
<!DOCTYPE html>
<meta charset="utf-8">
<title>Break opportunities after nowrap</title>
<link rel="author" title="Koji Ishii" href="mailto:kojii@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-text-3/#white-space-property">
<link rel="match" href="reference/white-space-wrap-after-nowrap-001-ref.html">
<meta name="assert" content="This test ensures that break opportunities after nowrap can break lines.">
<style>
div {
width: 10ch;
border: 2px solid blue;
line-height: 1;
}
.ideo > div {
width: 1em;
}
.normal {
white-space: normal;
}
.nowrap {
white-space: nowrap;
}
</style>
<body>
<section>
<div><span class="nowrap">12345</span> 67890</div>
<div><span class="nowrap">12345</span><span class="normal"> 67890</span></div>
<div><span class="nowrap">12345<span class="normal"> 67890</span></span></div>
<div class="nowrap">12345<span class="normal"> 67890</span></div>
<div class="nowrap"><span class="normal"><span class="nowrap">12345</span> </span>67890</div>
<div class="nowrap"><span class="normal"><span class="nowrap">12345 </span> </span>67890</div>
</section>
<section class="ideo">
<div><span class="nowrap"></span></div>
<div><span class="nowrap"></span><span class="normal"></span></div>
<div><span class="nowrap"><span class="normal"></span></span></div>
<div class="nowrap"><span class="normal"></span></div>
</section>
</body>
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