Commit 434458ed authored by Yoichi Osato's avatar Yoichi Osato Committed by Commit Bot

Fix line end caret w/o any following LayoutObject.

This patch fix ComputeInlineBoxPosition for the position that
points after line break and no LayoutObject after that by
fallback to previous position.
The fallback examples:
- "<br>|" to "|<br>"
- "<pre>foo\n|</pre>" to "<pre>foo|\n</pre>"

This change aligns legacy LocalCaretRect to NG.

Bug: 789870, 812535
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Icb6cd9b8bc58b56a04d01eb22bdd58bd16ad5a01
Reviewed-on: https://chromium-review.googlesource.com/1215506
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: default avatarXiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590199}
parent 445af7cd
...@@ -283,12 +283,7 @@ bool NeedsLineEndAdjustment( ...@@ -283,12 +283,7 @@ bool NeedsLineEndAdjustment(
// Returns the first InlineBoxPosition at next line of last InlineBoxPosition // Returns the first InlineBoxPosition at next line of last InlineBoxPosition
// in |layout_object| if it exists to avoid making InlineBoxPosition at end of // in |layout_object| if it exists to avoid making InlineBoxPosition at end of
// line. // line.
template <typename Strategy> InlineBoxPosition NextLinePositionOf(const LayoutText& layout_text) {
InlineBoxPosition NextLinePositionOf(
const PositionWithAffinityTemplate<Strategy>& adjusted) {
const PositionTemplate<Strategy>& position = adjusted.GetPosition();
const LayoutText& layout_text =
ToLayoutTextOrDie(*position.AnchorNode()->GetLayoutObject());
InlineTextBox* const last = layout_text.LastTextBox(); InlineTextBox* const last = layout_text.LastTextBox();
if (!last) if (!last)
return InlineBoxPosition(); return InlineBoxPosition();
...@@ -306,11 +301,26 @@ InlineBoxPosition NextLinePositionOf( ...@@ -306,11 +301,26 @@ InlineBoxPosition NextLinePositionOf(
return InlineBoxPosition(); return InlineBoxPosition();
} }
template <typename Strategy>
InlineBoxPosition ComputeInlineBoxPositionForLineEnd(
const PositionWithAffinityTemplate<Strategy>& adjusted) {
const LayoutText& layout_text = ToLayoutText(
*adjusted.GetPosition().AnchorNode()->GetLayoutObject());
const InlineBoxPosition next_line_position = NextLinePositionOf(layout_text);
if (next_line_position.inline_box)
return next_line_position;
// |adjusted| is after line end and no layout object after the position.
// Fall back to previous position.
DCHECK_GE(layout_text.TextLength(), 1u);
return ComputeInlineBoxPositionForTextNode(
&layout_text, layout_text.TextLength() - 1u, adjusted.Affinity());
}
template <typename Strategy> template <typename Strategy>
InlineBoxPosition ComputeInlineBoxPositionForInlineAdjustedPositionAlgorithm( InlineBoxPosition ComputeInlineBoxPositionForInlineAdjustedPositionAlgorithm(
const PositionWithAffinityTemplate<Strategy>& adjusted) { const PositionWithAffinityTemplate<Strategy>& adjusted) {
if (NeedsLineEndAdjustment(adjusted)) if (NeedsLineEndAdjustment(adjusted))
return NextLinePositionOf(adjusted); return ComputeInlineBoxPositionForLineEnd(adjusted);
const PositionTemplate<Strategy>& position = adjusted.GetPosition(); const PositionTemplate<Strategy>& position = adjusted.GetPosition();
DCHECK(!position.AnchorNode()->IsShadowRoot()) << adjusted; DCHECK(!position.AnchorNode()->IsShadowRoot()) << adjusted;
......
...@@ -698,11 +698,10 @@ TEST_P(ParameterizedLocalCaretRectTest, AfterLineBreak) { ...@@ -698,11 +698,10 @@ TEST_P(ParameterizedLocalCaretRectTest, AfterLineBreak) {
LocalCaretRect(second_br->GetLayoutObject(), LayoutRect(0, 10, 1, 10)), LocalCaretRect(second_br->GetLayoutObject(), LayoutRect(0, 10, 1, 10)),
LocalCaretRectOfPosition(PositionWithAffinity( LocalCaretRectOfPosition(PositionWithAffinity(
Position::AfterNode(*first_br), TextAffinity::kDownstream))); Position::AfterNode(*first_br), TextAffinity::kDownstream)));
EXPECT_EQ(LocalCaretRect(second_br->GetLayoutObject(), EXPECT_EQ(
LayoutNGEnabled() ? LayoutRect(0, 10, 1, 10) LocalCaretRect(second_br->GetLayoutObject(), LayoutRect(0, 10, 1, 10)),
: LayoutRect(0, 0, 0, 0)), LocalCaretRectOfPosition(PositionWithAffinity(
LocalCaretRectOfPosition(PositionWithAffinity( Position::AfterNode(*second_br), TextAffinity::kDownstream)));
Position::AfterNode(*second_br), TextAffinity::kDownstream)));
} }
TEST_P(ParameterizedLocalCaretRectTest, AfterLineBreakInPre) { TEST_P(ParameterizedLocalCaretRectTest, AfterLineBreakInPre) {
...@@ -716,10 +715,7 @@ TEST_P(ParameterizedLocalCaretRectTest, AfterLineBreakInPre) { ...@@ -716,10 +715,7 @@ TEST_P(ParameterizedLocalCaretRectTest, AfterLineBreakInPre) {
EXPECT_EQ(LocalCaretRect(foo->GetLayoutObject(), LayoutRect(0, 10, 1, 10)), EXPECT_EQ(LocalCaretRect(foo->GetLayoutObject(), LayoutRect(0, 10, 1, 10)),
LocalCaretRectOfPosition(PositionWithAffinity( LocalCaretRectOfPosition(PositionWithAffinity(
Position(foo, 4), TextAffinity::kDownstream))); Position(foo, 4), TextAffinity::kDownstream)));
// TODO(yoichio): Legacy should return valid rect: crbug.com/812535. EXPECT_EQ(LocalCaretRect(foo->GetLayoutObject(), LayoutRect(0, 10, 1, 10)),
EXPECT_EQ(LocalCaretRect(foo->GetLayoutObject(),
LayoutNGEnabled() ? LayoutRect(0, 10, 1, 10)
: LayoutRect(0, 0, 0, 0)),
LocalCaretRectOfPosition(PositionWithAffinity( LocalCaretRectOfPosition(PositionWithAffinity(
Position(foo, 5), TextAffinity::kDownstream))); Position(foo, 5), TextAffinity::kDownstream)));
} }
...@@ -738,9 +734,7 @@ TEST_P(ParameterizedLocalCaretRectTest, AfterLineBreakInPre2) { ...@@ -738,9 +734,7 @@ TEST_P(ParameterizedLocalCaretRectTest, AfterLineBreakInPre2) {
EXPECT_EQ(LocalCaretRect(br->GetLayoutObject(), LayoutRect(0, 10, 1, 10)), EXPECT_EQ(LocalCaretRect(br->GetLayoutObject(), LayoutRect(0, 10, 1, 10)),
LocalCaretRectOfPosition(PositionWithAffinity( LocalCaretRectOfPosition(PositionWithAffinity(
Position(foo, 4), TextAffinity::kDownstream))); Position(foo, 4), TextAffinity::kDownstream)));
EXPECT_EQ(LocalCaretRect(br->GetLayoutObject(), LayoutNGEnabled() EXPECT_EQ(LocalCaretRect(br->GetLayoutObject(), LayoutRect(0, 10, 1, 10)),
? LayoutRect(0, 10, 1, 10)
: LayoutRect(0, 0, 0, 0)),
LocalCaretRectOfPosition(PositionWithAffinity( LocalCaretRectOfPosition(PositionWithAffinity(
Position::AfterNode(*br), TextAffinity::kDownstream))); Position::AfterNode(*br), TextAffinity::kDownstream)));
} }
......
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