Commit 517b3e08 authored by Yoshifumi Inoue's avatar Yoshifumi Inoue Committed by Commit Bot

Get rid ot unused function TextIteratorBehavior::CollapseTrailingSpace()

This patch gets rid of unused function |CollapseTrailingSpace()| in
|TextIteratorBehavior| and related things for improving code health.

Note: This patch doesn't change behaviors.

Note: This patch exposes differences between legacy and LayoutNG about trailing
space to help implementing EditingNG. The legacy layout tree holds trailing
spaces, e.g. "ab  <br>cd", but LayoutNG doesn't.

Bug: 707656
Change-Id: Ib7c5c4a1be39bb7f50d0aa9378e8d1148e682d18
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2167633
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Auto-Submit: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762742}
parent e7fa06ad
......@@ -16,12 +16,6 @@ TextIteratorBehavior TextIteratorBehavior::Builder::Build() {
return behavior_;
}
TextIteratorBehavior::Builder&
TextIteratorBehavior::Builder::SetCollapseTrailingSpace(bool value) {
behavior_.values_.bits.collapse_trailing_space = value;
return *this;
}
TextIteratorBehavior::Builder&
TextIteratorBehavior::Builder::SetDoesNotBreakAtReplacedElement(bool value) {
behavior_.values_.bits.does_not_break_at_replaced_element = value;
......
......@@ -23,9 +23,6 @@ class CORE_EXPORT TextIteratorBehavior final {
bool operator==(const TextIteratorBehavior& other) const;
bool operator!=(const TextIteratorBehavior& other) const;
bool CollapseTrailingSpace() const {
return values_.bits.collapse_trailing_space;
}
bool DoesNotBreakAtReplacedElement() const {
return values_.bits.does_not_break_at_replaced_element;
}
......@@ -83,7 +80,6 @@ class CORE_EXPORT TextIteratorBehavior final {
union {
unsigned all;
struct {
bool collapse_trailing_space : 1;
bool does_not_break_at_replaced_element : 1;
bool emits_characters_between_all_visible_positions : 1;
bool emits_image_alt_text : 1;
......@@ -117,7 +113,6 @@ class CORE_EXPORT TextIteratorBehavior::Builder final {
TextIteratorBehavior Build();
Builder& SetCollapseTrailingSpace(bool);
Builder& SetDoesNotBreakAtReplacedElement(bool);
Builder& SetEmitsCharactersBetweenAllVisiblePositions(bool);
Builder& SetEmitsImageAltText(bool);
......
......@@ -11,33 +11,25 @@ namespace blink {
TEST(TextIteratorBehaviorTest, Basic) {
EXPECT_TRUE(TextIteratorBehavior() == TextIteratorBehavior());
EXPECT_FALSE(TextIteratorBehavior() != TextIteratorBehavior());
EXPECT_NE(
TextIteratorBehavior(),
TextIteratorBehavior::Builder().SetCollapseTrailingSpace(true).Build());
EXPECT_NE(
TextIteratorBehavior::Builder().SetCollapseTrailingSpace(true).Build(),
TextIteratorBehavior());
EXPECT_NE(
TextIteratorBehavior::Builder().SetCollapseTrailingSpace(true).Build(),
TextIteratorBehavior::Builder().SetEmitsImageAltText(true).Build());
EXPECT_NE(
TextIteratorBehavior::Builder().SetEmitsImageAltText(true).Build(),
TextIteratorBehavior::Builder().SetCollapseTrailingSpace(true).Build());
EXPECT_NE(TextIteratorBehavior(),
TextIteratorBehavior::Builder().SetForInnerText(true).Build());
EXPECT_NE(TextIteratorBehavior::Builder().SetForInnerText(true).Build(),
TextIteratorBehavior());
EXPECT_NE(TextIteratorBehavior::Builder().SetForInnerText(true).Build(),
TextIteratorBehavior::Builder().SetEmitsImageAltText(true).Build());
EXPECT_NE(TextIteratorBehavior::Builder().SetEmitsImageAltText(true).Build(),
TextIteratorBehavior::Builder().SetForInnerText(true).Build());
EXPECT_EQ(TextIteratorBehavior::Builder()
.SetCollapseTrailingSpace(true)
.SetForInnerText(true)
.SetEmitsImageAltText(true)
.Build(),
TextIteratorBehavior::Builder()
.SetEmitsImageAltText(true)
.SetCollapseTrailingSpace(true)
.SetForInnerText(true)
.Build());
}
TEST(TextIteratorBehaviorTest, Values) {
EXPECT_TRUE(TextIteratorBehavior::Builder()
.SetCollapseTrailingSpace(true)
.Build()
.CollapseTrailingSpace());
EXPECT_TRUE(TextIteratorBehavior::Builder()
.SetDoesNotBreakAtReplacedElement(true)
.Build()
......
......@@ -43,10 +43,6 @@
namespace blink {
namespace text_iterator_test {
TextIteratorBehavior CollapseTrailingSpaceBehavior() {
return TextIteratorBehavior::Builder().SetCollapseTrailingSpace(true).Build();
}
TextIteratorBehavior EmitsImageAltTextBehavior() {
return TextIteratorBehavior::Builder().SetEmitsImageAltText(true).Build();
}
......@@ -544,16 +540,41 @@ TEST_P(TextIteratorTest, RangeLengthWithFirstLetterMultipleLeadingSpaces) {
EXPECT_EQ(3, TestRangeLength("<p>^ foo|</p>"));
}
TEST_P(TextIteratorTest, TrainlingSpace) {
// text_content = "ab\ncd"
// offset mapping units:
// [0] I DOM:0-2 TC:0-2 "ab"
// [1] C DOM:2-4 TC:2-2 " " spaces after "ab"
// [2] I DOM:0-1 TC:2-3 <br>
// [3] I DOM:0-2 TC:3-5 "cd"
// Note: InlineTextBox has trailing spaces which we should get rid from
// inline layout tree as LayoutNG.
SetBodyContent("ab <br> cd");
EXPECT_EQ(LayoutNGEnabled() ? "[ab][\n][cd]" : "[ab ][\n][cd]",
Iterate<DOMTree>());
}
TEST_P(TextIteratorTest, WhitespaceCollapseForReplacedElements) {
static const char* body_content =
"<span>Some text </span> <input type='button' value='Button "
"text'/><span>Some more text</span>";
SetBodyContent(body_content);
EXPECT_EQ("[Some text ][][Some more text]",
Iterate<DOMTree>(CollapseTrailingSpaceBehavior()));
// text_content = "Some text \uFFFCSome more text"
// offset mapping units:
// [0] I DOM:0-10 TC:0-10 "Some text "
// [1] C DOM:0-1 TC:10-10 " " (A space between </span> and <input>
// [2] I DOM:0-1 TC:10-11 <input> as U+FFFC (ORC)
// [3] I DOM:0-14 TC:11-25 "Some more text"
// Note: InlineTextBox has a collapsed space which we should get rid from
// inline layout tree as LayoutNG.
EXPECT_EQ(LayoutNGEnabled() ? "[Some text ][][Some more text]"
: "[Some text ][ ][][Some more text]",
Iterate<DOMTree>());
// <input type=button> is not text control element
EXPECT_EQ("[Some text ][][Button text][Some more text]",
Iterate<FlatTree>(CollapseTrailingSpaceBehavior()));
EXPECT_EQ(LayoutNGEnabled()
? "[Some text ][][Button text][Some more text]"
: "[Some text ][ ][][Button text][Some more text]",
Iterate<FlatTree>());
}
TEST_P(TextIteratorTest, characterAt) {
......
......@@ -178,12 +178,9 @@ void TextIteratorTextNodeHandler::HandlePreFormattedTextNode() {
if (last_text_node_ended_with_collapsed_space_ &&
HasVisibleTextNode(layout_object)) {
if (!behavior_.CollapseTrailingSpace() ||
(offset_ > 0 && str[offset_ - 1] == ' ')) {
EmitChar16Before(kSpaceCharacter, offset_);
needs_handle_pre_formatted_text_node_ = true;
return;
}
EmitChar16Before(kSpaceCharacter, offset_);
needs_handle_pre_formatted_text_node_ = true;
return;
}
if (ShouldHandleFirstLetter(*layout_object)) {
LayoutTextFragment* remaining_text = ToLayoutTextFragment(layout_object);
......@@ -545,12 +542,7 @@ bool TextIteratorTextNodeHandler::ShouldFixLeadingWhiteSpaceForReplacedElement()
return false;
if (!last_text_node_ended_with_collapsed_space_)
return false;
if (!behavior_.CollapseTrailingSpace())
return true;
if (!text_node_)
return false;
const String str = text_node_->GetLayoutObject()->GetText();
return offset_ > 0 && str[offset_ - 1] == ' ';
return true;
}
bool TextIteratorTextNodeHandler::FixLeadingWhiteSpaceForReplacedElement() {
......
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