Skip to content

Commit

Permalink
A floating element can cause the following element’s text-indent to fail
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=263698
<rdar://problem/117538081>

Reviewed by Antti Koivisto.

text-indent should work even when the "first line" is completely taken by float(s).

  ______________________
 | ____________________ |
 ||                    ||
 ||    float box       ||
 ||____________________||
 |   this is still con- |
 |sidered first line and|
 |text-indent should    |
 |take affect.          |
 |                      |
 |                      |

text-indent logic consults now both previousLineEndsWithBreak and hasInlineContent.

* LayoutTests/fast/text/text-indent-with-wide-float-before-expected.html: Added.
* LayoutTests/fast/text/text-indent-with-wide-float-before.html: Added.
* Source/WebCore/layout/formattingContexts/inline/InlineContentBalancer.cpp:
(WebCore::Layout::InlineContentBalancer::initialize):
* Source/WebCore/layout/formattingContexts/inline/InlineFormattingContext.cpp:
(WebCore::Layout::InlineFormattingContext::layout):
(WebCore::Layout::InlineFormattingContext::lineLayout):
* Source/WebCore/layout/formattingContexts/inline/InlineLineBuilder.cpp:
(WebCore::Layout::LineBuilder::layoutInlineContent):
* Source/WebCore/layout/formattingContexts/inline/InlineLineTypes.h:
* Source/WebCore/layout/formattingContexts/inline/IntrinsicWidthHandler.cpp:
(WebCore::Layout::IntrinsicWidthHandler::computedIntrinsicWidthForConstraint):

Canonical link: https://commits.webkit.org/270238@main
  • Loading branch information
alanbaradlay committed Nov 5, 2023
1 parent 66793be commit e918ced
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<style>
.indent {
text-indent: 40px;
width: 400px;
font-family: Monospace;
margin-bottom: 1px;
}
</style>
<div class="indent">
Pass if this text is indented
</div>
<div class="indent">
Pass if this text is indented
</div>
<div class="indent">
Pass if this text is indented
</div>
29 changes: 29 additions & 0 deletions LayoutTests/fast/text/text-indent-with-wide-float-before.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<style>
.container {
width: 400px;
font-family: Monospace;
}

.indent {
text-indent: 40px;
}

.float_box {
float: left;
width: 400px;
height: 1px;
}
</style>
<div class="container indent">
Pass if this text is indented
</div>
<div class="container indent">
<div class=float_box></div>
Pass if this text is indented
</div>
<div class=container>
<div class=float_box></div>
<div class=indent>
Pass if this text is indented
</div>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ void InlineContentBalancer::initialize()

layoutRange.start = InlineFormattingUtils::leadingInlineItemPositionForNextLine(lineLayoutResult.inlineItemRange.end, previousLineEnd, layoutRange.end);
previousLineEnd = layoutRange.start;
previousLine = PreviousLine { lineIndex, lineLayoutResult.contentGeometry.trailingOverflowingContentWidth, !lineLayoutResult.inlineContent.isEmpty() && lineLayoutResult.inlineContent.last().isLineBreak(), lineLayoutResult.directionality.inlineBaseDirection, WTFMove(lineLayoutResult.floatContent.suspendedFloats) };
previousLine = PreviousLine { lineIndex, lineLayoutResult.contentGeometry.trailingOverflowingContentWidth, !lineLayoutResult.inlineContent.isEmpty() && lineLayoutResult.inlineContent.last().isLineBreak(), !lineLayoutResult.inlineContent.isEmpty(), lineLayoutResult.directionality.inlineBaseDirection, WTFMove(lineLayoutResult.floatContent.suspendedFloats) };
lineIndex++;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ InlineLayoutResult InlineFormattingContext::layout(const ConstraintsForInlineCon
}
auto lastLineIndex = lineDamage->start()->lineIndex - 1;
// FIXME: We should be able to extract the last line information and provide it to layout as "previous line" (ends in line break and inline direction).
return PreviousLine { lastLineIndex, { }, { }, { }, { } };
return PreviousLine { lastLineIndex, { }, { }, true, { }, { } };
};

if (root().style().textWrapMode() == TextWrapMode::Wrap && root().style().textWrapStyle() == TextWrapStyle::Balance) {
Expand Down Expand Up @@ -235,7 +235,8 @@ InlineLayoutResult InlineFormattingContext::lineLayout(AbstractLineBuilder& line
break;
}

previousLine = PreviousLine { lineIndex, lineLayoutResult.contentGeometry.trailingOverflowingContentWidth, !lineLayoutResult.inlineContent.isEmpty() && lineLayoutResult.inlineContent.last().isLineBreak(), lineLayoutResult.directionality.inlineBaseDirection, WTFMove(lineLayoutResult.floatContent.suspendedFloats) };
auto hasSeenInlineContent = previousLine ? previousLine->hasInlineContent || !lineLayoutResult.inlineContent.isEmpty() : !lineLayoutResult.inlineContent.isEmpty();
previousLine = PreviousLine { lineIndex, lineLayoutResult.contentGeometry.trailingOverflowingContentWidth, !lineLayoutResult.inlineContent.isEmpty() && lineLayoutResult.inlineContent.last().isLineBreak(), hasSeenInlineContent, lineLayoutResult.directionality.inlineBaseDirection, WTFMove(lineLayoutResult.floatContent.suspendedFloats) };
previousLineEnd = lineContentEnd;
lineLogicalTop = formattingUtils().logicalTopForNextLine(lineLayoutResult, lineLogicalRect, floatingContext);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ LineBuilder::LineBuilder(InlineFormattingContext& inlineFormattingContext, Horiz

LineLayoutResult LineBuilder::layoutInlineContent(const LineInput& lineInput, const std::optional<PreviousLine>& previousLine)
{
auto previousLineEndsWithLineBreak = !previousLine ? std::nullopt : std::make_optional(previousLine->endsWithLineBreak);
auto previousLineEndsWithLineBreak = !previousLine || !previousLine->hasInlineContent ? std::nullopt : std::make_optional(previousLine->endsWithLineBreak);
initialize(lineInput.initialLogicalRect, initialConstraintsForLine(lineInput.initialLogicalRect, previousLineEndsWithLineBreak), lineInput.needsLayoutRange, previousLine);
auto lineContent = placeInlineAndFloatContent(lineInput.needsLayoutRange);
auto result = m_line.close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ struct PreviousLine {
// Content width measured during line breaking (avoid double-measuring).
std::optional<InlineLayoutUnit> trailingOverflowingContentWidth { };
bool endsWithLineBreak { false };
bool hasInlineContent { false };
TextDirection inlineBaseDirection { TextDirection::LTR };
Vector<const Box*> suspendedFloats;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ InlineLayoutUnit IntrinsicWidthHandler::computedIntrinsicWidthForConstraint(Intr
// Support single line only.
mayCacheLayoutResult = MayCacheLayoutResult::No;
previousLineEnd = layoutRange.start;
previousLine = PreviousLine { lineIndex++, lineLayoutResult.contentGeometry.trailingOverflowingContentWidth, { }, { }, WTFMove(lineLayoutResult.floatContent.suspendedFloats) };
auto hasSeenInlineContent = previousLine ? previousLine->hasInlineContent || !lineLayoutResult.inlineContent.isEmpty() : !lineLayoutResult.inlineContent.isEmpty();
previousLine = PreviousLine { lineIndex++, lineLayoutResult.contentGeometry.trailingOverflowingContentWidth, !lineLayoutResult.inlineContent.isEmpty() && lineLayoutResult.inlineContent.last().isLineBreak(), hasSeenInlineContent, { }, WTFMove(lineLayoutResult.floatContent.suspendedFloats) };
}
return maximumContentWidth;
}
Expand Down

0 comments on commit e918ced

Please sign in to comment.