Commit 6d77a353 authored by Vadym Doroshenko's avatar Vadym Doroshenko Committed by Commit Bot

Revert "Fix caret rendering when at the end of a block after an ineditable inline"

This reverts commit e9aa20fd.

Reason for revert: this change breaks tests on win dbg bot

This is the first failure
https://ci.chromium.org/p/chromium/builders/ci/Win10%20Tests%20x64%20%28dbg%29/6437

Test InsertListCommandTest.CleanupNodeSameAsDestinationNode is definitely fails because of this patch (I've verified it locally on Windows).
The problem with this test that it's very deep recursion in function between functions
AdjustBlockFlowPositionToInline
ComputeInlineAdjustedPositionAlgorithm

and as a result stack overflow.


Original change's description:
> Fix caret rendering when at the end of a block after an ineditable inline
>
> AdjustBlockFlowPositionToInline() has an early-reject branch that avoids
> infinite recursion, introduced in crrev.com/664b6437.
>
> However, the early reject condition is too aggressive that, it also
> rejects some positions that don't enter an infinite recursion, like
> the one used in the new unit test added in this CL.
>
> As now AdjustBlockFlowPositionToInline() has a hard limit on recursion
> depth, this CL safely removes the over-aggressive reject condition, so
> that caret can be correctly rendered for more positions.
>
> Note: Unit test InlineBoxPositionTest.ComputeInlineBoxPositionMixedEditable
> uses another position that doesn't enter the infinite loop but is falsely
> rejected by the condition. This patch allows caret rendering also for that
> position, and therefore changes its test expectation.
>
> Bug: 936988
> Change-Id: Idef91ffaa412e67cddd5fcf0dd61f54055de7189
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1497466
> Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org>
> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
> Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#637311}

TBR=yosin@chromium.org,yoichio@chromium.org,xiaochengh@chromium.org

Change-Id: I6fcff118f27d633e627d5942671b04554fef283b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 936988
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1503579Reviewed-by: default avatarVadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#637676}
parent 48e417a8
...@@ -217,7 +217,8 @@ PositionWithAffinityTemplate<Strategy> AdjustBlockFlowPositionToInline( ...@@ -217,7 +217,8 @@ PositionWithAffinityTemplate<Strategy> AdjustBlockFlowPositionToInline(
} }
const PositionTemplate<Strategy>& upstream_equivalent = const PositionTemplate<Strategy>& upstream_equivalent =
UpstreamIgnoringEditingBoundaries(position); UpstreamIgnoringEditingBoundaries(position);
if (upstream_equivalent == position) if (upstream_equivalent == position ||
DownstreamIgnoringEditingBoundaries(upstream_equivalent) == position)
return PositionWithAffinityTemplate<Strategy>(); return PositionWithAffinityTemplate<Strategy>();
return ComputeInlineAdjustedPositionAlgorithm( return ComputeInlineAdjustedPositionAlgorithm(
......
...@@ -49,15 +49,12 @@ TEST_F(InlineBoxPositionTest, ComputeInlineBoxPositionMixedEditable) { ...@@ -49,15 +49,12 @@ TEST_F(InlineBoxPositionTest, ComputeInlineBoxPositionMixedEditable) {
SetBodyContent( SetBodyContent(
"<div contenteditable id=sample>abc<input contenteditable=false></div>"); "<div contenteditable id=sample>abc<input contenteditable=false></div>");
Element* const sample = GetDocument().getElementById("sample"); Element* const sample = GetDocument().getElementById("sample");
Element* const input = GetDocument().QuerySelector("input");
const InlineBox* const input_wrapper_box =
ToLayoutBox(input->GetLayoutObject())->InlineBoxWrapper();
const InlineBoxPosition& actual = ComputeInlineBoxPosition( const InlineBoxPosition& actual = ComputeInlineBoxPosition(
PositionWithAffinity(Position::LastPositionInNode(*sample))); PositionWithAffinity(Position::LastPositionInNode(*sample)));
// Should not be in infinite-loop // Should not be in infinite-loop
EXPECT_EQ(input_wrapper_box, actual.inline_box); EXPECT_EQ(nullptr, actual.inline_box);
EXPECT_EQ(1, actual.offset_in_box); EXPECT_EQ(0, actual.offset_in_box);
} }
// http://crbug.com/841363 // http://crbug.com/841363
......
...@@ -972,21 +972,4 @@ TEST_P(ParameterizedLocalCaretRectTest, AfterCollapsedWhiteSpaceInRTLText) { ...@@ -972,21 +972,4 @@ TEST_P(ParameterizedLocalCaretRectTest, AfterCollapsedWhiteSpaceInRTLText) {
PositionWithAffinity(position, TextAffinity::kDownstream))); PositionWithAffinity(position, TextAffinity::kDownstream)));
} }
// https://crbug.com/936988
TEST_P(ParameterizedLocalCaretRectTest, AfterIneditableInline) {
// For LayoutNG, we also enable EditingNG to test NG caret rendering.
ScopedEditingNGForTest editing_ng(LayoutNGEnabled());
LoadAhem();
InsertStyleElement("div { font: 10px/10px Ahem }");
SetBodyContent(
"<div contenteditable><span contenteditable=\"false\">foo</span></div>");
const Element* div = GetDocument().QuerySelector("div");
const Node* text = div->firstChild()->firstChild();
const Position position = Position::LastPositionInNode(*div);
EXPECT_EQ(LocalCaretRect(text->GetLayoutObject(), LayoutRect(30, 0, 1, 10)),
LocalCaretRectOfPosition(PositionWithAffinity(position)));
}
} // namespace blink } // namespace blink
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