Skip to content

Commit b6df426

Browse files
alanbaradlayalanbaradlay
committed
REGRESSION(261836@main): Incorrect caret placement after hitting enter/backspace/enter combo in long text
https://bugs.webkit.org/show_bug.cgi?id=254989 <rdar://problem/108215532> Reviewed by Antti Koivisto. This bugs caused by an incorrectly computed (damaged) line index which caused us exiting inline layout too early (and not producing content for the new newline). With partial layout we 1. first compute the damaged line index which is the entry point for the subsequent inline layout. 2. run inline layout until we see no change in generated display boxes anymore (in many cases the damage only affects a range of lines and not the full set) With the following content: First line 1\n Second line 2\n Third line 3\n When a new \n is inserted between the last \n and [3], we compute the damage position by looking at the _start_ of the previous sibling. In this case the previous sibling is "First line 1\nSecond line 2\nThird line 3\n" which means we damage the content starting from line #0. The subsequent inline layout starts generating display boxes starting from line #0 and bails out at the end of line #1 since the set of display boxes are getting generated match what we had before. This fix ensures that when we insert some content, the damaged line index is computed based on the insertion point (end of the previous sibling). * Source/WebCore/layout/formattingContexts/inline/invalidation/InlineInvalidation.cpp: (WebCore::Layout::damagedLineIndex): (WebCore::Layout::inlineItemPositionForDamagedContentPosition): (WebCore::Layout::InlineInvalidation::textInserted): (WebCore::Layout::InlineInvalidation::inlineLevelBoxInserted): Canonical link: https://commits.webkit.org/263113@main
1 parent d289e00 commit b6df426

File tree

3 files changed

+48
-9
lines changed

3 files changed

+48
-9
lines changed

‎LayoutTests/fast/inline/partial-layout-with-enter-backspace-enter-combo-expected.txt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
1. Move caret to the end of the editable content.
2+
2. Hit enter-backspace-enter combo.
3+
PASS if caret moves.
4+
5+
ABC ABC ABC
6+
PASS

‎LayoutTests/fast/inline/partial-layout-with-enter-backspace-enter-combo.html‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<style>
2+
textarea {
3+
width: 40px;
4+
height: 500px;
5+
}
6+
</style>
7+
<pre>
8+
1. Move caret to the end of the editable content.
9+
2. Hit enter-backspace-enter combo.
10+
PASS if caret moves.
11+
</pre>
12+
<textarea id=focus_this>ABC ABC ABC</textarea>
13+
<pre id=result></pre>
14+
<script>
15+
if (window.testRunner)
16+
testRunner.dumpAsText();
17+
focus_this.focus();
18+
document.execCommand('selectAll', false, null);
19+
document.getSelection().collapseToEnd();
20+
document.execCommand('insertText', false, "\n");
21+
window.getSelection().modify("extend", "backward", "character");
22+
document.execCommand('forwardDelete');
23+
document.execCommand('insertText', false, "\nPASS");
24+
result.innerText = focus_this.value;
25+
</script>

‎Source/WebCore/layout/formattingContexts/inline/invalidation/InlineInvalidation.cpp‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ void InlineInvalidation::styleChanged(const Box& layoutBox, const RenderStyle& o
5353

5454
struct DamagedContent {
5555
const Box& layoutBox;
56-
size_t offset { 0 };
56+
// Only text type of boxes may have offset. No offset also simply points to the end of the layout box.
57+
std::optional<size_t> offset { };
5758
enum class Type : uint8_t {
5859
Insertion,
5960
Removal
@@ -96,12 +97,12 @@ static std::optional<size_t> damagedLineIndex(std::optional<DamagedContent> dama
9697
if (damagedContent->type == DamagedContent::Type::Insertion || !damagedDisplayBox.lineIndex())
9798
return { damagedDisplayBox.lineIndex() };
9899
if (!displayBoxes[damagedDisplayBoxIndex - 1].isRootInlineBox()) {
99-
// There's more content in front.
100+
// There's more content in front, it's safe to stay on the current line.
100101
ASSERT(displayBoxes[damagedDisplayBoxIndex - 1].lineIndex() == damagedDisplayBox.lineIndex());
101102
return { damagedDisplayBox.lineIndex() };
102103
}
103-
auto lineWillBeVisuallyEmpty = !damagedDisplayBox.isText() || damagedDisplayBox.text().start() == damagedContent->offset;
104-
return { lineWillBeVisuallyEmpty ? damagedDisplayBox.lineIndex() - 1 : damagedDisplayBox.lineIndex() };
104+
auto damagedContentIsFirstContentOnLine = !damagedDisplayBox.isText() || (damagedContent->offset && damagedDisplayBox.text().start() == *damagedContent->offset);
105+
return { damagedContentIsFirstContentOnLine ? damagedDisplayBox.lineIndex() - 1 : damagedDisplayBox.lineIndex() };
105106
};
106107

107108
auto leadingIndexForDisplayBox = *lastDisplayBoxIndex;
@@ -111,6 +112,9 @@ static std::optional<size_t> damagedLineIndex(std::optional<DamagedContent> dama
111112
}
112113

113114
if (is<InlineTextBox>(damagedContent->layoutBox)) {
115+
if (!damagedContent->offset)
116+
return candidateLineIndex(leadingIndexForDisplayBox);
117+
114118
for (auto index = leadingIndexForDisplayBox; index > 0; --index) {
115119
auto& displayBox = displayBoxes[index];
116120
if (displayBox.isRootInlineBox() || (displayBox.isInlineBox() && !displayBox.isFirstForLayoutBox())) {
@@ -124,7 +128,7 @@ static std::optional<size_t> damagedLineIndex(std::optional<DamagedContent> dama
124128
ASSERT(displayBox.isTextOrSoftLineBreak());
125129
// Find out which display box has the damaged offset position.
126130
auto startOffset = displayBox.text().start();
127-
if (startOffset <= damagedContent->offset) {
131+
if (startOffset <= *damagedContent->offset) {
128132
// FIXME: Add support for leading inline box deletion too.
129133
return candidateLineIndex(index);
130134
}
@@ -217,7 +221,11 @@ static std::optional<InlineItemPosition> inlineItemPositionForDamagedContentPosi
217221
if (&candidateInlineItem.layoutBox() != &damagedContent.layoutBox || !is<InlineTextItem>(candidateInlineItem))
218222
return candidatePosition;
219223
auto& inlineTextItem = downcast<InlineTextItem>(candidateInlineItem);
220-
if (inlineTextItem.start() + candidatePosition.offset <= damagedContent.offset)
224+
if (!damagedContent.offset) {
225+
// When damage points to "after" the layout box, whatever InlineItem we found is surely before the damage.
226+
return candidatePosition;
227+
}
228+
if (inlineTextItem.start() + candidatePosition.offset <= *damagedContent.offset)
221229
return candidatePosition;
222230
// The damage offset is in front of the first display box we managed to find for this layout box.
223231
// Let's adjust the candidate position by moving it over to the damaged offset.
@@ -228,7 +236,7 @@ static std::optional<InlineItemPosition> inlineItemPositionForDamagedContentPosi
228236
return { };
229237
}
230238
auto& previousInlineItem = downcast<InlineTextItem>(inlineItems[index]);
231-
if (previousInlineItem.start() <= damagedContent.offset)
239+
if (previousInlineItem.start() <= *damagedContent.offset)
232240
return InlineItemPosition { index, { } };
233241
}
234242
return { };
@@ -348,7 +356,7 @@ void InlineInvalidation::textInserted(const InlineTextBox& newOrDamagedInlineTex
348356
damagedLine = DamagedLine { };
349357
// New text box got inserted. Let's damage existing content starting from the previous sibling.
350358
if (auto* previousSibling = newOrDamagedInlineTextBox.previousInFlowSibling())
351-
damagedLine = leadingInlineItemPositionByDamagedBox({ *previousSibling, { } }, m_inlineItems, displayBoxes());
359+
damagedLine = leadingInlineItemPositionByDamagedBox({ *previousSibling }, m_inlineItems, displayBoxes());
352360
}
353361

354362
updateInlineDamage(!damagedLine ? InlineDamage::Type::Invalid : InlineDamage::Type::NeedsContentUpdateAndLineLayout, damagedLine, offset ? ShouldApplyRangeLayout::No : ShouldApplyRangeLayout::Yes);
@@ -382,7 +390,7 @@ void InlineInvalidation::inlineLevelBoxInserted(const Box& layoutBox)
382390
damagedLine = DamagedLine { };
383391
// New box got inserted. Let's damage existing content starting from the previous sibling.
384392
if (auto* previousSibling = layoutBox.previousInFlowSibling())
385-
damagedLine = leadingInlineItemPositionByDamagedBox({ *previousSibling, { } }, m_inlineItems, displayBoxes());
393+
damagedLine = leadingInlineItemPositionByDamagedBox({ *previousSibling }, m_inlineItems, displayBoxes());
386394
}
387395
updateInlineDamage(!damagedLine ? InlineDamage::Type::Invalid : InlineDamage::Type::NeedsContentUpdateAndLineLayout, damagedLine, ShouldApplyRangeLayout::Yes);
388396
}

0 commit comments

Comments
 (0)