From b2ec60c7d5b8ab1ded9edfae217285f4059aa28d Mon Sep 17 00:00:00 2001 From: Alan Baradlay Date: Sat, 16 Mar 2024 16:47:10 -0700 Subject: [PATCH] [IFC][Intrinsic width] Incorrect shrink-to-fit box logical width when negative text-indent is present https://bugs.webkit.org/show_bug.cgi?id=271113 Reviewed by Antti Koivisto. Negative text-indent value could confuse the min/max inline size computation as running layout with 0 constraint may produce wider content than running layout with infinite constraint. Consider the following case:
some content
With infinite constraint this content produce only one line with the line box width of 0 (assume the measured content width of "some content" is < 100px) It simply means that [some content] visually overflows on the left due to this implicit negative content margin. _ some content |_| However when computing the minimum content size we have to take into account all the possible soft wrap opportunities with the constraint value of 0. Now we end up constructing 2 lines where the second line has 0px used text-indent. _ some |_| ________ |content | -------- which clearly computes a content value > 0px. Let's fix this by making sure minimum width is never larger than maximum width. This behavior also seems to match with other rendering engines. * LayoutTests/fast/text/min-max-content-negative-text-indent-expected.html: Added. * LayoutTests/fast/text/min-max-content-negative-text-indent.html: Added. * Source/WebCore/layout/formattingContexts/inline/InlineFormattingContext.cpp: (WebCore::Layout::InlineFormattingContext::minimumMaximumContentSize): (WebCore::Layout::InlineFormattingContext::minimumContentSize): Canonical link: https://commits.webkit.org/276246@main --- ...content-negative-text-indent-expected.html | 19 +++++++++++++++ .../min-max-content-negative-text-indent.html | 19 +++++++++++++++ .../inline/InlineFormattingContext.cpp | 23 +++++++++++++++++++ 3 files changed, 61 insertions(+) create mode 100644 LayoutTests/fast/text/min-max-content-negative-text-indent-expected.html create mode 100644 LayoutTests/fast/text/min-max-content-negative-text-indent.html diff --git a/LayoutTests/fast/text/min-max-content-negative-text-indent-expected.html b/LayoutTests/fast/text/min-max-content-negative-text-indent-expected.html new file mode 100644 index 000000000000..db5adac10189 --- /dev/null +++ b/LayoutTests/fast/text/min-max-content-negative-text-indent-expected.html @@ -0,0 +1,19 @@ + +
X
+
XX
+
XXX
+
XXXX
+
XXXXX
+
XXXXX
+
XXXXX
+
XXXXX
+
XXXXXXX
\ No newline at end of file diff --git a/LayoutTests/fast/text/min-max-content-negative-text-indent.html b/LayoutTests/fast/text/min-max-content-negative-text-indent.html new file mode 100644 index 000000000000..fb2dc3389080 --- /dev/null +++ b/LayoutTests/fast/text/min-max-content-negative-text-indent.html @@ -0,0 +1,19 @@ + +
XXXX XXXXX
+
XXXX XXXXX
+
XXXX XXXXX
+
XXXX XXXXX
+
XXXX XXXXX
+
XXXX XXXXX
+
XXXX XXXXX
+
XXXX XXXXX
+
XXXX XXXXX
diff --git a/Source/WebCore/layout/formattingContexts/inline/InlineFormattingContext.cpp b/Source/WebCore/layout/formattingContexts/inline/InlineFormattingContext.cpp index 75b4447857d1..d0fe4ec458fe 100644 --- a/Source/WebCore/layout/formattingContexts/inline/InlineFormattingContext.cpp +++ b/Source/WebCore/layout/formattingContexts/inline/InlineFormattingContext.cpp @@ -44,6 +44,7 @@ #include "IntrinsicWidthHandler.h" #include "LayoutBox.h" #include "LayoutContext.h" +#include "LayoutDescendantIterator.h" #include "LayoutElementBox.h" #include "LayoutInitialContainingBlock.h" #include "LayoutInlineTextBox.h" @@ -180,6 +181,28 @@ std::pair InlineFormattingContext::minimumMaximumContent minimumContentSize = minimumContentSize.value_or(0.f); maximumContentSize = maximumContentSize.value_or(0.f); } +#ifndef NDEBUG + // FIXME: "Nominally, the smallest size a box could take that doesn’t lead to overflow that could be avoided by choosing a larger size. + // Formally, the size of the box when sized under a min-content constraint" + // 'nominally' seems to overrule 'formally' when inline content has negative text indent. + // This also undermines the idea of computing min/max values independently. + if (*minimumContentSize > *maximumContentSize) { + auto hasNegativeImplicitMargin = [](auto& style) { + return (style.textIndent().isFixed() && style.textIndent().value() < 0) || style.wordSpacing() < 0 || style.letterSpacing() < 0; + }; + auto contentHasNegativeImplicitMargin = hasNegativeImplicitMargin(root().style()); + if (!contentHasNegativeImplicitMargin) { + for (auto& layoutBox : descendantsOfType(root())) { + contentHasNegativeImplicitMargin = hasNegativeImplicitMargin(layoutBox.style()); + if (contentHasNegativeImplicitMargin) + break; + } + } + ASSERT(contentHasNegativeImplicitMargin); + } +#endif + minimumContentSize = std::min(*minimumContentSize, *maximumContentSize); + inlineContentCache.setMinimumContentSize(*minimumContentSize); inlineContentCache.setMaximumContentSize(*maximumContentSize); return { ceiledLayoutUnit(*minimumContentSize), ceiledLayoutUnit(*maximumContentSize) };