Skip to content

GitLab

T
tangled
Commit 7282b926 authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot
Browse files
Options

Use |HasOverflow| to determine needs to add ellipsis 

This patch changes |NGInlineLayoutAlgorithm| to use
|HasOverflow|, a flag set by |NGLineBreaker|, instead of
comparing the size with the available size.

|NGLineBreaker| allows lines to overflow up to |LayoutUnit::
Epsilon|, because authors expect monospace to fit to the
exact width, but rounding errors may exceed by |Epsilon|.

This patch fixes not to add ellipsis when this occurs.

Bug: 1134483
Change-Id: I893d20cdbc77af5de26dafa149a5713442751266
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2448069Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813578}
parent edea8b30
Show whitespace changes
Inline Side-by-side
Showing with 33 additions and 3 deletions
third_party/blink/renderer/core/layout/ng/inline/ng_inline_layout_algorithm.cc
View file @ 7282b926
...@@ -334,8 +334,7 @@ void NGInlineLayoutAlgorithm::CreateLine( ...@@ -334,8 +334,7 @@ void NGInlineLayoutAlgorithm::CreateLine(
} }
// Truncate the line if 'text-overflow: ellipsis' is set, or for line-clamp. // Truncate the line if 'text-overflow: ellipsis' is set, or for line-clamp.
if (UNLIKELY((inline_size > if (UNLIKELY((line_info->HasOverflow() &&
line_info->AvailableWidth() - line_info->TextIndent() &&
node_.GetLayoutBlockFlow()->ShouldTruncateOverflowingText()) || node_.GetLayoutBlockFlow()->ShouldTruncateOverflowingText()) ||
ConstraintSpace().LinesUntilClamp() == 1)) { ConstraintSpace().LinesUntilClamp() == 1)) {
NGLineTruncator truncator(*line_info); NGLineTruncator truncator(*line_info);
......
third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker_test.cc
View file @ 7282b926
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_testing.h" #include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_testing.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_break_token.h" #include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_break_token.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_cursor.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_node.h" #include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_node.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.h" #include "third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.h"
#include "third_party/blink/renderer/core/layout/ng/layout_ng_block_flow.h" #include "third_party/blink/renderer/core/layout/ng/layout_ng_block_flow.h"
...@@ -42,6 +43,7 @@ class NGLineBreakerTest : public NGLayoutTest { ...@@ -42,6 +43,7 @@ class NGLineBreakerTest : public NGLayoutTest {
Vector<std::pair<String, unsigned>> BreakLines( Vector<std::pair<String, unsigned>> BreakLines(
NGInlineNode node, NGInlineNode node,
LayoutUnit available_width, LayoutUnit available_width,
void (*callback)(const NGLineBreaker&, const NGLineInfo&) = nullptr,
bool fill_first_space_ = false) { bool fill_first_space_ = false) {
DCHECK(node); DCHECK(node);
...@@ -66,6 +68,8 @@ class NGLineBreakerTest : public NGLayoutTest { ...@@ -66,6 +68,8 @@ class NGLineBreakerTest : public NGLayoutTest {
line_opportunity, leading_floats, 0u, line_opportunity, leading_floats, 0u,
break_token.get(), &exclusion_space); break_token.get(), &exclusion_space);
line_breaker.NextLine(&line_info); line_breaker.NextLine(&line_info);
if (callback)
callback(line_breaker, line_info);
trailing_whitespaces_.push_back( trailing_whitespaces_.push_back(
line_breaker.TrailingWhitespaceForTesting()); line_breaker.TrailingWhitespaceForTesting());
...@@ -92,6 +96,33 @@ class NGLineBreakerTest : public NGLayoutTest { ...@@ -92,6 +96,33 @@ class NGLineBreakerTest : public NGLayoutTest {
namespace { namespace {
TEST_F(NGLineBreakerTest, FitWithEpsilon) {
LoadAhem();
NGInlineNode node = CreateInlineNode(R"HTML(
<!DOCTYPE html>
<style>
#container {
font: 10px/1 Ahem;
width: 49.99px;
overflow: hidden;
text-overflow: ellipsis;
}
</style>
<div id=container>00000</div>
)HTML");
auto lines = BreakLines(
node, LayoutUnit::FromFloatRound(50 - LayoutUnit::Epsilon()),
[](const NGLineBreaker& line_breaker, const NGLineInfo& line_info) {
EXPECT_FALSE(line_info.HasOverflow());
});
EXPECT_EQ(1u, lines.size());
// Make sure ellipsizing code use the same |HasOverflow|.
NGInlineCursor cursor(*node.GetLayoutBlockFlow());
for (; cursor; cursor.MoveToNext())
EXPECT_FALSE(cursor.Current().IsEllipsis());
}
TEST_F(NGLineBreakerTest, SingleNode) { TEST_F(NGLineBreakerTest, SingleNode) {
LoadAhem(); LoadAhem();
NGInlineNode node = CreateInlineNode(R"HTML( NGInlineNode node = CreateInlineNode(R"HTML(
...@@ -514,7 +545,7 @@ TEST_P(NGTrailingSpaceWidthTest, TrailingSpaceWidth) { ...@@ -514,7 +545,7 @@ TEST_P(NGTrailingSpaceWidthTest, TrailingSpaceWidth) {
R"HTML(</div> R"HTML(</div>
)HTML"); )HTML");
BreakLines(node, LayoutUnit(50), true); BreakLines(node, LayoutUnit(50), nullptr, true);
if (first_should_hang_trailing_space_) { if (first_should_hang_trailing_space_) {
EXPECT_EQ(first_hang_width_, LayoutUnit(10) * data.trailing_space_width); EXPECT_EQ(first_hang_width_, LayoutUnit(10) * data.trailing_space_width);
} else { } else {
......
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