Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[IFC][Partial layout] Incorrect damage line index may cause double in…
…line items

https://bugs.webkit.org/show_bug.cgi?id=256020
<rdar://108558653>

Reviewed by Antti Koivisto.

When the damaged content is at the beginning of the line, we mark the previous line as
the entry point for the subsequent partial layout. It is not only a correctness requirement but also it provides a more
convenient starting point for line layout/InlineItemBuilder to process the inline content.

Layout::damagedLineIndex failed to recognize a leading display box due to collapsed whitespace content.
Through some further content mutation, it lead to duplicated inline items (hard line breaks).

* LayoutTests/fast/inline/contenteditable-with-leading-whitespace-crash-expected.txt: Added.
* LayoutTests/fast/inline/contenteditable-with-leading-whitespace-crash.html: Added.
* Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp:
(WebCore::Layout::InlineItemsBuilder::build):
* Source/WebCore/layout/formattingContexts/inline/invalidation/InlineInvalidation.cpp:
(WebCore::Layout::damagedLineIndex):

Canonical link: https://commits.webkit.org/263455@main
  • Loading branch information
alanbaradlay committed Apr 27, 2023
1 parent 9ac904f commit 44b5f74
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 2 deletions.
@@ -0,0 +1,3 @@
PASS if no crash.


@@ -0,0 +1,20 @@
<style>
div {
display: contents;
}

.flex_box {
display: flex;
}
</style>PASS if no crash.<div class=flex_box onfocusin="justify()" contenteditable="true" draggable="true"> <details id=details></details></div>
<script>
if (window.testRunner)
testRunner.dumpAsText();
document.body.offsetHeight;
details.outerText = "some content";
document.defaultView.find("some");

function justify() {
document.execCommand("justifyCenter", false, null);
}
</script>
Expand Up @@ -106,14 +106,20 @@ void InlineItemsBuilder::build(InlineItemPosition startPosition)
adjustInlineFormattingStateWithNewInlineItems();

#if ASSERT_ENABLED
// Check if we've got matching inline box start/end pairs.
// Check if we've got matching inline box start/end pairs and unique inline level items (non-text, non-inline box items).
size_t inlineBoxStart = 0;
size_t inlineBoxEnd = 0;
auto inlineLevelItems = HashSet<const Box*> { };
for (auto& inlineItem : m_formattingState.inlineItems()) {
if (inlineItem.isInlineBoxStart())
++inlineBoxStart;
else if (inlineItem.isInlineBoxEnd())
++inlineBoxEnd;
else {
auto hasToBeUniqueLayoutBox = inlineItem.isBox() || inlineItem.isFloat() || inlineItem.isHardLineBreak();
if (hasToBeUniqueLayoutBox)
ASSERT(inlineLevelItems.add(&inlineItem.layoutBox()).isNewEntry);
}
}
ASSERT(inlineBoxStart == inlineBoxEnd);
#endif
Expand Down
Expand Up @@ -101,7 +101,7 @@ static std::optional<size_t> damagedLineIndex(std::optional<DamagedContent> dama
ASSERT(displayBoxes[damagedDisplayBoxIndex - 1].lineIndex() == damagedDisplayBox.lineIndex());
return { damagedDisplayBox.lineIndex() };
}
auto damagedContentIsFirstContentOnLine = !damagedDisplayBox.isText() || (damagedContent->offset && damagedDisplayBox.text().start() == *damagedContent->offset);
auto damagedContentIsFirstContentOnLine = !damagedDisplayBox.isText() || (damagedContent->offset && (!*damagedContent->offset || damagedDisplayBox.text().start() == *damagedContent->offset));
return { damagedContentIsFirstContentOnLine ? damagedDisplayBox.lineIndex() - 1 : damagedDisplayBox.lineIndex() };
};

Expand Down

0 comments on commit 44b5f74

Please sign in to comment.