Skip to content

Commit

Permalink
REGRESSION: A floating element can cause the latter half of a hyphena…
Browse files Browse the repository at this point in the history
…ted word to disappear

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

Reviewed by Antti Koivisto.

It's okay to not being able to advance on the inline content even when it is partial (either through hyphenation or arbitrary breaking position) when
there is an intrusive float on the line.

* LayoutTests/fast/inline/intrusive-float-with-no-available-space-and-partial-content-expected.html: Added.
* LayoutTests/fast/inline/intrusive-float-with-no-available-space-and-partial-content.html: Added.
* Source/WebCore/layout/floats/FloatingContext.cpp:
(WebCore::Layout::FloatingContext::constraints const):
* Source/WebCore/layout/formattingContexts/inline/InlineContentBalancer.cpp:
(WebCore::Layout::InlineContentBalancer::initialize):
* Source/WebCore/layout/formattingContexts/inline/InlineFormattingContext.cpp:
(WebCore::Layout::InlineFormattingContext::lineLayout):
* Source/WebCore/layout/formattingContexts/inline/InlineFormattingUtils.cpp:
(WebCore::Layout::InlineFormattingUtils::leadingInlineItemPositionForNextLine):
* Source/WebCore/layout/formattingContexts/inline/InlineFormattingUtils.h:
* Source/WebCore/layout/formattingContexts/inline/InlineLineBuilder.cpp:
(WebCore::Layout::LineBuilder::placeInlineAndFloatContent):
* Source/WebCore/layout/formattingContexts/inline/IntrinsicWidthHandler.cpp:
(WebCore::Layout::IntrinsicWidthHandler::computedIntrinsicWidthForConstraint):

Canonical link: https://commits.webkit.org/273836@main
  • Loading branch information
alanbaradlay committed Jan 31, 2024
1 parent 0806bfc commit 9f7e229
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 13 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<style>
.content {
width: 200px;
border: 1px solid black;
word-break: break-all;
font-family: Monospace;
font-size: 16px;
}

.before-float {
float: right;
width: 20px;
height: 20px;
background-color: green;
}

.after-float {
float: left;
width: 200px;
background-color: green;
height: 40px;
}
</style>
<div class=content>PASSifthistextisfu<div class=before-float></div><div class=after-float></div>llyvisible</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<style>
.content {
width: 200px;
border: 1px solid black;
word-break: break-all;
font-family: Monospace;
font-size: 16px;
}

.before-float {
float: right;
width: 20px;
height: 20px;
background-color: green;
}

.after-float {
float: left;
width: 200px;
background-color: green;
height: 40px;
}
</style>
<div class=content><div class=before-float></div><div class=after-float></div>PASSifthistextisfullyvisible</div>
3 changes: 2 additions & 1 deletion Source/WebCore/layout/floats/FloatingContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,8 @@ FloatingContext::Constraints FloatingContext::constraints(LayoutUnit candidateTo
if (!shape->lineOverlapsShapeMarginBounds(positionInShape, candidateHeight))
return { };

auto segment = shape->getExcludedInterval(positionInShape, candidateHeight);
// PolygonShape gets confused when passing in 0px height interval at vertices.
auto segment = shape->getExcludedInterval(positionInShape, std::max(candidateHeight, 1_lu));
if (!segment.isValid)
return { };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ void InlineContentBalancer::initialize()
if (numberOfVisibleLinesAllowed && (lineIndex + 1 >= numberOfVisibleLinesAllowed))
break;

layoutRange.start = InlineFormattingUtils::leadingInlineItemPositionForNextLine(lineLayoutResult.inlineItemRange.end, previousLineEnd, layoutRange.end);
layoutRange.start = InlineFormattingUtils::leadingInlineItemPositionForNextLine(lineLayoutResult.inlineItemRange.end, previousLineEnd, !lineLayoutResult.floatContent.hasIntrusiveFloat.isEmpty(), layoutRange.end);
previousLineEnd = layoutRange.start;
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 @@ -265,7 +265,7 @@ InlineLayoutResult InlineFormattingContext::lineLayout(AbstractLineBuilder& line
updateInlineLayoutStateWithLineLayoutResult(lineLayoutResult, lineLogicalRect, floatingContext);

auto lineContentEnd = lineLayoutResult.inlineItemRange.end;
leadingInlineItemPosition = InlineFormattingUtils::leadingInlineItemPositionForNextLine(lineContentEnd, previousLineEnd, needsLayoutRange.end);
leadingInlineItemPosition = InlineFormattingUtils::leadingInlineItemPositionForNextLine(lineContentEnd, previousLineEnd, !lineLayoutResult.floatContent.hasIntrusiveFloat.isEmpty(), needsLayoutRange.end);
auto isLastLine = leadingInlineItemPosition == needsLayoutRange.end && lineLayoutResult.floatContent.suspendedFloats.isEmpty();
if (isLastLine) {
layoutResult.range = !isPartialLayout ? InlineLayoutResult::Range::Full : InlineLayoutResult::Range::FullFromDamage;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,18 +284,22 @@ InlineLayoutUnit InlineFormattingUtils::horizontalAlignmentOffset(const RenderSt
return { };
}

InlineItemPosition InlineFormattingUtils::leadingInlineItemPositionForNextLine(InlineItemPosition lineContentEnd, std::optional<InlineItemPosition> previousLineTrailingInlineItemPosition, InlineItemPosition layoutRangeEnd)
InlineItemPosition InlineFormattingUtils::leadingInlineItemPositionForNextLine(InlineItemPosition lineContentEnd, std::optional<InlineItemPosition> previousLineContentEnd, bool lineHasIntrusiveFloat, InlineItemPosition layoutRangeEnd)
{
if (!previousLineTrailingInlineItemPosition)
if (!previousLineContentEnd)
return lineContentEnd;
if (previousLineTrailingInlineItemPosition->index < lineContentEnd.index || (previousLineTrailingInlineItemPosition->index == lineContentEnd.index && previousLineTrailingInlineItemPosition->offset < lineContentEnd.offset)) {
if (previousLineContentEnd->index < lineContentEnd.index || (previousLineContentEnd->index == lineContentEnd.index && previousLineContentEnd->offset < lineContentEnd.offset)) {
// Either full or partial advancing.
return lineContentEnd;
}
if (previousLineTrailingInlineItemPosition->index == lineContentEnd.index && !previousLineTrailingInlineItemPosition->offset && !lineContentEnd.offset) {
// Can't mangage to put any content on line (most likely due to floats). Note that this only applies to full content.
if (lineContentEnd == *previousLineContentEnd && lineHasIntrusiveFloat) {
// Couldn't manage to put any content on line due to floats.
return lineContentEnd;
}
if (lineContentEnd == layoutRangeEnd) {
// End of content.
return layoutRangeEnd;
}
// This looks like a partial content and we are stuck. Let's force-move over to the next inline item.
// We certainly lose some content, but we would be busy looping otherwise.
ASSERT_NOT_REACHED();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class InlineFormattingUtils {

static InlineLayoutUnit horizontalAlignmentOffset(const RenderStyle& rootStyle, InlineLayoutUnit contentLogicalRight, InlineLayoutUnit lineLogicalRight, InlineLayoutUnit hangingTrailingWidth, const Line::RunList& runs, bool isLastLine, std::optional<TextDirection> inlineBaseDirectionOverride = std::nullopt);

static InlineItemPosition leadingInlineItemPositionForNextLine(InlineItemPosition lineContentEnd, std::optional<InlineItemPosition> previousLineTrailingInlineItemPosition, InlineItemPosition layoutRangeEnd);
static InlineItemPosition leadingInlineItemPositionForNextLine(InlineItemPosition lineContentEnd, std::optional<InlineItemPosition> previousLineContentEnd, bool lineHasIntrusiveFloat, InlineItemPosition layoutRangeEnd);

InlineLayoutUnit inlineItemWidth(const InlineItem&, InlineLayoutUnit contentLogicalLeft, bool useFirstLineStyle) const;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -462,11 +462,17 @@ LineContent LineBuilder::placeInlineAndFloatContent(const InlineItemRange& needs
layoutInlineAndFloatContent();

auto computePlacedInlineItemRange = [&] {
lineContent.range = { needsLayoutRange.start, { needsLayoutRange.startIndex() + placedInlineItemCount, { } } };
if (!placedInlineItemCount || placedInlineItemCount == m_placedFloats.size() || !lineContent.partialTrailingContentLength)
lineContent.range = { needsLayoutRange.start, needsLayoutRange.start };

if (!placedInlineItemCount)
return;

if (placedInlineItemCount == m_placedFloats.size() || !lineContent.partialTrailingContentLength) {
lineContent.range.end = { needsLayoutRange.startIndex() + placedInlineItemCount, { } };
return;
}

auto trailingInlineItemIndex = lineContent.range.end.index - 1;
auto trailingInlineItemIndex = needsLayoutRange.startIndex() + placedInlineItemCount - 1;
auto overflowingInlineTextItemLength = downcast<InlineTextItem>(m_inlineItemList[trailingInlineItemIndex]).length();
ASSERT(lineContent.partialTrailingContentLength && lineContent.partialTrailingContentLength < overflowingInlineTextItemLength);
lineContent.range.end = { trailingInlineItemIndex, overflowingInlineTextItemLength - lineContent.partialTrailingContentLength };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ InlineLayoutUnit IntrinsicWidthHandler::computedIntrinsicWidthForConstraint(Intr
if (lineEndsWithLineBreak)
contentWidthBetweenLineBreaks = { std::max(contentWidthBetweenLineBreaks.maximum, contentWidthBetweenLineBreaks.current), { } };

layoutRange.start = InlineFormattingUtils::leadingInlineItemPositionForNextLine(lineLayoutResult.inlineItemRange.end, previousLineEnd, layoutRange.end);
layoutRange.start = InlineFormattingUtils::leadingInlineItemPositionForNextLine(lineLayoutResult.inlineItemRange.end, previousLineEnd, !lineLayoutResult.floatContent.hasIntrusiveFloat.isEmpty(), layoutRange.end);
if (layoutRange.isEmpty()) {
auto cacheLineBreakingResultForSubsequentLayoutIfApplicable = [&] {
m_maximumIntrinsicWidthResultForSingleLine = { };
Expand Down

0 comments on commit 9f7e229

Please sign in to comment.