Skip to content

Commit

Permalink
[IFC] Negative letter-spacing breaks "-webkit-box-decoration-break: c…
Browse files Browse the repository at this point in the history
…lone"

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

Reviewed by Antti Koivisto.

Trailing decoration was not taken into account when accumulating text content width for content with negative letter spacing.
Let's move out m_contentLogicalWidth logic from [add new], [expand last with/without letter spacing] branches so that introducing a new
branch would not result in the same error.

* LayoutTests/fast/inline/webkit-box-decoration-clone-with-negative-letter-spacing-expected.html: Added.
* LayoutTests/fast/inline/webkit-box-decoration-clone-with-negative-letter-spacing.html: Added.
* Source/WebCore/layout/formattingContexts/inline/InlineLine.cpp:
(WebCore::Layout::Line::appendTextContent):

Canonical link: https://commits.webkit.org/263540@main
  • Loading branch information
alanbaradlay committed Apr 30, 2023
1 parent 2c27397 commit f5213fd
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 21 deletions.
@@ -0,0 +1,17 @@
<style>
div {
font-size: 20px;
font-family: Ahem;
letter-spacing: -1px;
padding-left: 30px;
padding-right: 30px;
background-color: cyan;
display: inline-block;
}
</style>
<div>PASS if</div><br>
<div>all lines</div><br>
<div>have</div><br>
<div>padding</div><br>
<div>on the</div><br>
<div>right</div>
@@ -0,0 +1,15 @@
<style>
div {
width: 240px;
font-size: 20px;
font-family: Ahem;
letter-spacing: -1px;
}
span {
-webkit-box-decoration-break: clone;
padding-left: 30px;
padding-right: 30px;
background-color: cyan;
}
</style>
<div><span>PASS if all lines have padding on the right</span></div>
44 changes: 23 additions & 21 deletions Source/WebCore/layout/formattingContexts/inline/InlineLine.cpp
Expand Up @@ -457,6 +457,7 @@ void Line::appendTextContent(const InlineTextItem& inlineTextItem, const RenderS
}();
auto oldContentLogicalWidth = contentLogicalWidth();
auto runHasHangablePunctuationStart = isFirstFormattedLine() && TextUtil::hasHangablePunctuationStart(inlineTextItem, style) && !lineHasVisuallyNonEmptyContent();
auto contentLogicalRight = InlineLayoutUnit { };
if (needsNewRun) {
// Note, negative word spacing may cause glyph overlap.
auto runLogicalLeft = [&] {
Expand All @@ -466,31 +467,32 @@ void Line::appendTextContent(const InlineTextItem& inlineTextItem, const RenderS
}();
m_runs.append({ inlineTextItem, style, runLogicalLeft, logicalWidth });
// Note that the _content_ logical right may be larger than the _run_ logical right.
auto contentLogicalRight = runLogicalLeft + logicalWidth + m_clonedEndDecorationWidthForInlineBoxRuns;
m_contentLogicalWidth = std::max(oldContentLogicalWidth, contentLogicalRight);
} else if (style.letterSpacing() >= 0) {
auto& lastRun = m_runs.last();
lastRun.expand(inlineTextItem, logicalWidth);
// Ensure that property values that act like negative margin are not making the line wider.
m_contentLogicalWidth = std::max(oldContentLogicalWidth, lastRun.logicalRight() + m_clonedEndDecorationWidthForInlineBoxRuns);
contentLogicalRight = runLogicalLeft + logicalWidth;
} else {
auto& lastRun = m_runs.last();
ASSERT(lastRun.isText());
// Negative letter spacing should only shorten the content to the boundary of the previous run.
// FIXME: We may need to traverse all the way to the previous non-text run (or even across inline boxes).
auto contentWidthWithoutLastTextRun = [&] {
if (style.fontCascade().wordSpacing() >= 0)
return m_contentLogicalWidth - std::max(0.f, lastRun.logicalWidth());
// FIXME: Let's see if we need to optimize for this is the rare case of both letter and word spacing being negative.
auto rightMostPosition = InlineLayoutUnit { };
for (auto& run : makeReversedRange(m_runs))
rightMostPosition = std::max(rightMostPosition, run.logicalRight());
return std::max(0.f, rightMostPosition);
}();
auto lastRunLogicalRight = lastRun.logicalRight();
lastRun.expand(inlineTextItem, logicalWidth);
m_contentLogicalWidth = std::max(contentWidthWithoutLastTextRun, lastRunLogicalRight + logicalWidth);
if (style.letterSpacing() >= 0) {
lastRun.expand(inlineTextItem, logicalWidth);
contentLogicalRight = lastRun.logicalRight();
} else {
auto contentWidthWithoutLastTextRun = [&] {
// FIXME: We may need to traverse all the way to the previous non-text run (or even across inline boxes).
if (style.fontCascade().wordSpacing() >= 0)
return m_contentLogicalWidth - std::max(0.f, lastRun.logicalWidth());
// FIXME: Let's see if we need to optimize for this is the rare case of both letter and word spacing being negative.
auto rightMostPosition = InlineLayoutUnit { };
for (auto& run : makeReversedRange(m_runs))
rightMostPosition = std::max(rightMostPosition, run.logicalRight());
return std::max(0.f, rightMostPosition);
}();
auto lastRunLogicalRight = lastRun.logicalRight();
lastRun.expand(inlineTextItem, logicalWidth);
// Negative letter spacing should only shorten the content to the boundary of the previous run.
contentLogicalRight = std::max(contentWidthWithoutLastTextRun, lastRunLogicalRight + logicalWidth);
}
}
// Ensure that property values that act like negative margin are not making the line wider.
m_contentLogicalWidth = std::max(oldContentLogicalWidth, contentLogicalRight + m_clonedEndDecorationWidthForInlineBoxRuns);

auto lastRunIndex = m_runs.size() - 1;
m_trailingSoftHyphenWidth = { };
Expand Down

0 comments on commit f5213fd

Please sign in to comment.