Skip to content

Commit

Permalink
[IFC][Intrinsic width] Incorrect shrink-to-fit box logical width when…
Browse files Browse the repository at this point in the history
… negative text-indent is present

https://bugs.webkit.org/show_bug.cgi?id=271113
<rdar://problem/124784190>

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:

<div style="text-indent: -100px;">some content</div>

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
  • Loading branch information
alanbaradlay committed Mar 16, 2024
1 parent a492c4f commit b2ec60c
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<style>
div {
width: min-content;
height: 50px;
background-color: green;
color: transparent;
font-family: Ahem;
font-size: 20px;
}
</style>
<div>X</div>
<div>XX</div>
<div>XXX</div>
<div>XXXX</div>
<div>XXXXX</div>
<div>XXXXX</div>
<div>XXXXX</div>
<div>XXXXX</div>
<div>XXXXXXX</div>
19 changes: 19 additions & 0 deletions LayoutTests/fast/text/min-max-content-negative-text-indent.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<style>
div {
width: min-content;
height: 50px;
background-color: green;
color: transparent;
font-family: Ahem;
font-size: 20px;
}
</style>
<div style="text-indent: -180px;">XXXX XXXXX</div>
<div style="text-indent: -160px;">XXXX XXXXX</div>
<div style="text-indent: -140px;">XXXX XXXXX</div>
<div style="text-indent: -120px;">XXXX XXXXX</div>
<div style="text-indent: -100px;">XXXX XXXXX</div>
<div style="text-indent: -80px;">XXXX XXXXX</div>
<div style="text-indent: -60px;">XXXX XXXXX</div>
<div style="text-indent: 0px;">XXXX XXXXX</div>
<div style="text-indent: 60px;">XXXX XXXXX</div>
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -180,6 +181,28 @@ std::pair<LayoutUnit, LayoutUnit> 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<Box>(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) };
Expand Down

0 comments on commit b2ec60c

Please sign in to comment.