Skip to content

GitLab

T
tangled
Commit 88e45427 authored by Javier Fernandez's avatar Javier Fernandez Committed by Commit Bot
Browse files
Options

[css-text] A leading white-space should break before handling overflow 

Leading white-spaces are indeed breaking opportunities that should
prevent, if there are no other css properties forcing it, breaking text
in the middle of a word when honoring the word-wrap/overflow-wrap CSS
property.

We are doing so if the leading white-space sequence is longer than 1
character, but when we have a single leading white-space, we are missing
that breaking opportunity and we may lead to cases, like the one
described in the bug.

The root cause of the issue with single leading white-space breaking
opportunities is that the RewindToMidWordBreak expects certain width to
be committed in order to choose opportunities in previous runs if none
of the ones detected by the ICU LazyLineBreakIterator prevents the
overflow.

However, this breaking opportunity should be considered together
with other provided by the word-break CSS property (eg, break-word or
break-all), as it was agreed in the discussion [1] with the CSS WG.

This CL solves the issue identifying the single leading white-space
braking opportunity in a new class field flag, and using it to consider
this opportunity inside the mid-word breaking logic, or prevent to run
it completely in the cases where 'break-all' is not set.

This is basically a reland of 6ea2a2e7
but with some changes to avoid regressions like the one reported in
issue #866109.

[1] https://github.com/w3c/csswg-drafts/issues/2907

Bug: 854624
Change-Id: I1cc0f55050d54ea1e76c655cf6b3ef8bcc0b0e2c
Reviewed-on: https://chromium-review.googlesource.com/c/1209745
Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596406}
parent 292dbc0c
Hide whitespace changes
Inline Side-by-side
Showing with 197 additions and 2 deletions
third_party/WebKit/LayoutTests/external/wpt/css/css-text/overflow-wrap/overflow-wrap-break-word-004.html 0 → 100644
View file @ 88e45427
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Text Test: overflow-wrap: break-word</title>
<link rel="author" title="Javier Fernandez Garcia-Boente" href="mailto:jfernandez@igalia.com">
<link rel="help" href="https://drafts.csswg.org/css-text-3/#valdef-overflow-wrap-break-word">
<meta name="flags" content="ahem">
<link rel="match" href="reference/overflow-wrap-break-word-001-ref.html">
<meta name="assert" content="A Single leading white-space constitutes a soft breaking opportunity, honoring the 'white-space: pre-wrap' property, that must prevent the word to be broken.">
<style>
div {
position: relative;
font-size: 20px;
font-family: Ahem;
}
.red {
position: absolute;
background: green;
color: red;
width: 100px;
height: 100px;
z-index: -1;
}
.test {
color: green;
line-height: 1em;
width: 5ch;
white-space: pre-wrap;
overflow-wrap: break-word;
}
</style>
<body>
<p>Test passes if there is a <strong>filled green square</strong> and <strong>no red</strong>.</p>
<div class="red"><br>XXXXX</div>
<div class="test"> XXXXX </div>
</body>
third_party/WebKit/LayoutTests/external/wpt/css/css-text/white-space/pre-wrap-015.html 0 → 100644
View file @ 88e45427
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Text Test: white-space: pre-wrap</title>
<link rel="author" title="Javier Fernandez Garcia-Boente" href="mailto:jfernandez@igalia.com">
<link rel="help" href="https://drafts.csswg.org/css-text-3/#valdef-white-space-pre-wrap">
<link rel="help" href="https://drafts.csswg.org/css-text-3/#valdef-word-break-break-all">
<meta name="flags" content="ahem">
<link rel="match" href="reference/pre-wrap-001-ref.html">
<meta name="assert" content="The text is broken at the end of the space between the two words, never before, so it hangs and cause an overflow">
<style>
div {
position: relative;
font: 20px/1 Ahem;
}
.ref {
position: absolute;
color: red;
z-index: -1;
}
.test {
color: green;
width: 20px;
white-space: pre-wrap;
word-break: break-word;
}
</style>
<body>
<p>Test passes if there is a <strong>filled green square</strong> and <strong>no red</strong>.</p>
<div class="ref">X<span style="color: green">X</span><br>X<span style="color: green">X</span></div>
<div class="test">X X</div>
</body>
third_party/WebKit/LayoutTests/external/wpt/css/css-text/word-break/reference/word-break-break-all-010-ref.html 0 → 100644
View file @ 88e45427
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Text Reference File</title>
<link rel="author" title="Florian Rivoal" href="http://florian.rivoal.net/">
<style>
div {
position: relative;
width: 100px;
height: 100px;
background: green;
}
</style>
<body>
<p>Test passes if there is a <strong>filled green square</strong> and <strong>no red</strong>.</p>
<div></div>
</body>
third_party/WebKit/LayoutTests/external/wpt/css/css-text/word-break/reference/word-break-break-all-014-ref.html 0 → 100644
View file @ 88e45427
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Text Test: overflow-wrap: break-all</title>
<link rel="author" title="Javier Fernandez Garcia-Boente" href="mailto:jfernandez@igalia.com">
<p>Test passes if 2 icons are rendered in a row, matching the reference file.</p>
<div>💖<br>💔</div>
third_party/WebKit/LayoutTests/external/wpt/css/css-text/word-break/word-break-break-all-010.html 0 → 100644
View file @ 88e45427
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Text Test: overflow-wrap: break-word</title>
<link rel="author" title="Javier Fernandez Garcia-Boente" href="mailto:jfernandez@igalia.com">
<link rel="help" href="https://drafts.csswg.org/css-text-3/#valdef-word-break-break-all">
<meta name="flags" content="ahem">
<link rel="match" href="reference/word-break-break-all-010-ref.html">
<meta name="assert" content="The word is broken even if pre-wrap provides a former breaking opportunity in leading white-space.">
<style>
div {
position: relative;
font-size: 20px;
font-family: Ahem;
}
.red {
position: absolute;
white-space: pre;
background: green;
color: red;
width: 100px;
height: 100px;
z-index: -1;
}
.test {
color: green;
line-height: 1em;
width: 5ch;
white-space: pre-wrap;
word-break: break-all;
}
</style>
<body>
<p>Test passes if there is a <strong>filled green square</strong> and <strong>no red</strong>.</p>
<div class="red"> XXXX<br>X</div>
<div class="test"> XXXXX</div>
</body>
third_party/WebKit/LayoutTests/external/wpt/css/css-text/word-break/word-break-break-all-011.html 0 → 100644
View file @ 88e45427
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Text Test: overflow-wrap: break-word</title>
<link rel="author" title="Javier Fernandez Garcia-Boente" href="mailto:jfernandez@igalia.com">
<link rel="help" href="https://drafts.csswg.org/css-text-3/#valdef-word-break-break-all">
<meta name="flags" content="ahem">
<link rel="match" href="reference/word-break-break-all-010-ref.html">
<meta name="assert" content="A single leading white-space should account as soft breaking opportunity, honoring the 'white-space: pre-wrap', on top to the ones provided by 'word-break: break-all'.">
<style>
div {
position: relative;
font-size: 20px;
font-family: Ahem;
}
.red {
position: absolute;
background: green;
color: red;
width: 100px;
height: 100px;
z-index: -1;
}
.test {
color: green;
background: green;
line-height: 1em;
width: 1ch;
white-space: pre-wrap;
word-break: break-all;
}
</style>
<body>
<p>Test passes if there is a <strong>filled green square</strong> and <strong>no red</strong>.</p>
<div class="red">X<br>X<br>X</div>
<div class="test"> XX</div>
</body>
third_party/WebKit/LayoutTests/external/wpt/css/css-text/word-break/word-break-break-all-014.html 0 → 100644
View file @ 88e45427
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Text Test: overflow-wrap: break-all</title>
<link rel="author" title="Javier Fernandez Garcia-Boente" href="mailto:jfernandez@igalia.com">
<link rel="help" href="https://drafts.csswg.org/css-text-3/#valdef-word-break-break-all">
<meta name="flags" content="ahem">
<link rel="match" href="reference/word-break-break-all-014-ref.html">
<meta name="assert" content="The text is wrapped after the first character and no unicode unit is broken.">
<style>
div {
width: 1px;
word-break: break-all;
}
</style>
<p>Test passes if 2 icons are rendered in a row, matching the reference file.</p>
<div>💖💔</div>
third_party/blink/renderer/core/layout/line/breaking_context_inline_headers.h
View file @ 88e45427
......@@ -207,6 +207,7 @@ class BreakingContext {
bool at_start_;
bool ignoring_spaces_;
bool current_character_is_space_;
bool single_leading_space_;
bool applied_start_width_;
bool include_end_width_;
bool auto_wrap_;
......@@ -377,6 +378,8 @@ inline void BreakingContext::InitializeForCurrentObject() {
// pre-wrap">text <span><span> text</span>' does not collapse.
if (collapse_white_space_ && !ComputedStyle::CollapseWhiteSpace(last_ws_))
current_character_is_space_ = false;
single_leading_space_ = false;
}
inline void BreakingContext::Increment() {
......@@ -835,7 +838,8 @@ ALWAYS_INLINE bool BreakingContext::RewindToFirstMidWordBreak(
collapse_white_space_);
// If the first break opportunity doesn't fit, and if there's a break
// opportunity in previous runs, break at the opportunity.
if (!width_.FitsOnLine(width) && width_.CommittedWidth())
if (!width_.FitsOnLine(width) &&
(width_.CommittedWidth() || single_leading_space_))
return false;
return RewindToMidWordBreak(word_measurement, end, width);
}
......@@ -1043,6 +1047,15 @@ inline bool BreakingContext::HandleText(WordMeasurements& word_measurements,
UChar c = current_.Current();
SetCurrentCharacterIsSpace(c);
// Auto-wrapping text should not wrap in the middle of a word if it has
// an opportunity to break at a leading white-space.
// TODO (jfernandez): This change is questionable, but it's required to
// achieve the expected behavior for 'break-word' (cases 2.1 and 2.2), while
// keeping current behavior for 'break-all' (cases 4.1 and 4.2)
// https://github.com/w3c/csswg-drafts/issues/2907
if (single_leading_space_)
can_break_mid_word = break_all;
if (!collapse_white_space_ || !current_character_is_space_) {
line_info_.SetEmpty(false);
width_.SetTrailingWhitespaceWidth(0);
......@@ -1346,9 +1359,12 @@ inline void BreakingContext::PrepareForNextCharacter(
start_of_ignored_spaces_.SetOffset(current_.Offset());
}
if (!current_character_is_space_ && previous_character_is_space) {
if (auto_wrap_ && current_style_->BreakOnlyAfterWhiteSpace())
if (auto_wrap_ && current_style_->BreakOnlyAfterWhiteSpace()) {
line_break_.MoveTo(current_.GetLineLayoutItem(), current_.Offset(),
current_.NextBreakablePosition());
if (current_.Offset() == 1)
single_leading_space_ = true;
}
}
if (collapse_white_space_ && current_character_is_space_ && !ignoring_spaces_)
trailing_objects_.SetTrailingWhitespace(
......
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