Diff
Modified: trunk/Source/WebCore/ChangeLog (288054 => 288055)
--- trunk/Source/WebCore/ChangeLog 2022-01-15 13:32:05 UTC (rev 288054)
+++ trunk/Source/WebCore/ChangeLog 2022-01-15 13:36:49 UTC (rev 288055)
@@ -1,3 +1,26 @@
+2022-01-15 Alan Bujtas <[email protected]>
+
+ [LFC][IFC] imported/w3c/web-platform-tests/css/css-text/word-break/word-break-normal-ar-000.html fails with incorrect run position
+ https://bugs.webkit.org/show_bug.cgi?id=235011
+
+ Reviewed by Antti Koivisto.
+
+ IFC (inherited from simple line layout) uses this technique of measuring the content with the trailing whitespace
+ and then simply subtract the whitespace width. It enables us to keep pushing content to the line without re-measuring it each time.
+ (the "non-whitespace + whitespace" pattern is extremely common for IFC content).
+ However in some cases when the trailing whitespace is trimmed, subtracting the trimmed width from the
+ content width instead of measuring it produces a visually incorrect result.
+ This patch fixes the most obvious cases when the incorrect width turns into an offset for the RTL content.
+
+ * layout/formattingContexts/inline/InlineLine.cpp:
+ (WebCore::Layout::Line::TrimmableTrailingContent::remove):
+ (WebCore::Layout::Line::Run::removeTrailingLetterSpacing):
+ (WebCore::Layout::Line::Run::removeTrailingWhitespace):
+ * layout/formattingContexts/inline/InlineLine.h:
+ * layout/formattingContexts/inline/text/TextUtil.cpp:
+ (WebCore::Layout::TextUtil::width):
+ * layout/formattingContexts/inline/text/TextUtil.h:
+
2022-01-14 Antoine Quint <[email protected]>
Setting `content: normal` on a ::marker should make computed style return resolved values
Modified: trunk/Source/WebCore/layout/formattingContexts/inline/InlineLine.cpp (288054 => 288055)
--- trunk/Source/WebCore/layout/formattingContexts/inline/InlineLine.cpp 2022-01-15 13:32:05 UTC (rev 288054)
+++ trunk/Source/WebCore/layout/formattingContexts/inline/InlineLine.cpp 2022-01-15 13:36:49 UTC (rev 288055)
@@ -343,7 +343,9 @@
m_hangingTrailingContent.add(inlineTextItem, logicalWidth);
} else {
m_hangingTrailingContent.reset();
- m_trimmableTrailingContent.addFullyTrimmableContent(lastRunIndex, contentLogicalWidth() - oldContentLogicalWidth);
+ auto trimmableWidth = logicalWidth;
+ auto trimmableContentOffset = (contentLogicalWidth() - oldContentLogicalWidth) - trimmableWidth;
+ m_trimmableTrailingContent.addFullyTrimmableContent(lastRunIndex, trimmableContentOffset, trimmableWidth);
}
m_trailingSoftHyphenWidth = { };
} else {
@@ -450,11 +452,12 @@
{
}
-void Line::TrimmableTrailingContent::addFullyTrimmableContent(size_t runIndex, InlineLayoutUnit trimmableWidth)
+void Line::TrimmableTrailingContent::addFullyTrimmableContent(size_t runIndex, InlineLayoutUnit trimmableContentOffset, InlineLayoutUnit trimmableWidth)
{
// Any subsequent trimmable whitespace should collapse to zero advanced width and ignored at ::appendTextContent().
ASSERT(!m_hasFullyTrimmableContent);
- m_fullyTrimmableWidth = trimmableWidth;
+ m_fullyTrimmableWidth = trimmableContentOffset + trimmableWidth;
+ m_trimmableContentOffset = trimmableContentOffset;
// Note that just because the trimmable width is 0 (font-size: 0px), it does not mean we don't have a trimmable trailing content.
m_hasFullyTrimmableContent = true;
m_firstTrimmableRunIndex = m_firstTrimmableRunIndex.value_or(runIndex);
@@ -481,12 +484,12 @@
auto& trimmableRun = m_runs[*m_firstTrimmableRunIndex];
ASSERT(trimmableRun.isText());
+ auto trimmedWidth = m_trimmableContentOffset;
if (m_hasFullyTrimmableContent)
- trimmableRun.removeTrailingWhitespace();
+ trimmedWidth += trimmableRun.removeTrailingWhitespace();
if (m_partiallyTrimmableWidth)
- trimmableRun.removeTrailingLetterSpacing();
+ trimmedWidth += trimmableRun.removeTrailingLetterSpacing();
- auto trimmableWidth = width();
// When the trimmable run is followed by some non-content runs, we need to adjust their horizontal positions.
// e.g. <div>text is followed by trimmable content <span> </span></div>
// When the [text...] run is trimmed (trailing whitespace is removed), both "<span>" and "</span>" runs
@@ -495,7 +498,7 @@
for (auto index = *m_firstTrimmableRunIndex + 1; index < m_runs.size(); ++index) {
auto& run = m_runs[index];
ASSERT(run.isWordBreakOpportunity() || run.isLineSpanningInlineBoxStart() || run.isInlineBoxStart() || run.isInlineBoxEnd() || run.isLineBreak());
- run.moveHorizontally(-trimmableWidth);
+ run.moveHorizontally(-trimmedWidth);
}
if (!trimmableRun.textContent()->length) {
// This trimmable run is fully collapsed now (e.g. <div><img> <span></span></div>).
@@ -503,7 +506,7 @@
m_runs.remove(*m_firstTrimmableRunIndex);
}
reset();
- return trimmableWidth;
+ return trimmedWidth;
}
InlineLayoutUnit Line::TrimmableTrailingContent::removePartiallyTrimmableContent()
@@ -625,6 +628,7 @@
if (!whitespaceType) {
m_trailingWhitespace = { };
m_textContent->length += inlineTextItem.length();
+ m_lastNonWhitespaceContentStart = inlineTextItem.start();
return;
}
auto whitespaceWidth = !m_trailingWhitespace ? logicalWidth : m_trailingWhitespace->width + logicalWidth;
@@ -644,23 +648,39 @@
return InlineLayoutUnit { letterSpacing() };
}
-void Line::Run::removeTrailingLetterSpacing()
+InlineLayoutUnit Line::Run::removeTrailingLetterSpacing()
{
ASSERT(hasTrailingLetterSpacing());
- shrinkHorizontally(trailingLetterSpacing());
+ auto trailingWidth = trailingLetterSpacing();
+ shrinkHorizontally(trailingWidth);
ASSERT(logicalWidth() > 0 || (!logicalWidth() && letterSpacing() >= static_cast<float>(intMaxForLayoutUnit)));
+ return trailingWidth;
}
-void Line::Run::removeTrailingWhitespace()
+InlineLayoutUnit Line::Run::removeTrailingWhitespace()
{
ASSERT(m_trailingWhitespace);
// According to https://www.w3.org/TR/css-text-3/#white-space-property matrix
// Trimmable whitespace is always collapsible so the length of the trailing trimmable whitespace is always 1 (or non-existent).
- ASSERT(m_textContent->length);
+ ASSERT(m_textContent && m_textContent->length);
constexpr size_t trailingTrimmableContentLength = 1;
+
+ auto trimmedWidth = m_trailingWhitespace->width;
+ if (m_lastNonWhitespaceContentStart && inlineDirection() == TextDirection::RTL) {
+ // While LTR content could also suffer from slightly incorrect content width after trimming trailing whitespace (see TextUtil::width)
+ // it hardly produces visually observable result.
+ // FIXME: This may still incorrectly leave some content on the line (vs. re-measuring also at ::expand).
+ auto& inlineTextBox = downcast<InlineTextBox>(*m_layoutBox);
+ auto startPosition = *m_lastNonWhitespaceContentStart;
+ auto endPosition = m_textContent->start + m_textContent->length;
+ RELEASE_ASSERT(startPosition < endPosition - trailingTrimmableContentLength);
+ if (inlineTextBox.content()[endPosition - 1] == space)
+ trimmedWidth = TextUtil::trailingWhitespaceWidth(inlineTextBox, m_style.fontCascade(), startPosition, endPosition);
+ }
m_textContent->length -= trailingTrimmableContentLength;
- shrinkHorizontally(m_trailingWhitespace->width);
m_trailingWhitespace = { };
+ shrinkHorizontally(trimmedWidth);
+ return trimmedWidth;
}
}
Modified: trunk/Source/WebCore/layout/formattingContexts/inline/InlineLine.h (288054 => 288055)
--- trunk/Source/WebCore/layout/formattingContexts/inline/InlineLine.h 2022-01-15 13:32:05 UTC (rev 288054)
+++ trunk/Source/WebCore/layout/formattingContexts/inline/InlineLine.h 2022-01-15 13:36:49 UTC (rev 288055)
@@ -143,11 +143,11 @@
bool hasCollapsibleTrailingWhitespace() const { return m_trailingWhitespace && (m_trailingWhitespace->type == TrailingWhitespace::Type::Collapsible || hasCollapsedTrailingWhitespace()); }
bool hasCollapsedTrailingWhitespace() const { return m_trailingWhitespace && m_trailingWhitespace->type == TrailingWhitespace::Type::Collapsed; }
static std::optional<TrailingWhitespace::Type> trailingWhitespaceType(const InlineTextItem&);
- void removeTrailingWhitespace();
+ InlineLayoutUnit removeTrailingWhitespace();
bool hasTrailingLetterSpacing() const;
InlineLayoutUnit trailingLetterSpacing() const;
- void removeTrailingLetterSpacing();
+ InlineLayoutUnit removeTrailingLetterSpacing();
Type m_type { Type::Text };
const Box* m_layoutBox { nullptr };
@@ -157,6 +157,7 @@
InlineDisplay::Box::Expansion m_expansion;
UBiDiLevel m_bidiLevel { UBIDI_DEFAULT_LTR };
std::optional<TrailingWhitespace> m_trailingWhitespace { };
+ std::optional<size_t> m_lastNonWhitespaceContentStart { };
std::optional<Text> m_textContent;
};
using RunList = Vector<Run, 10>;
@@ -185,7 +186,7 @@
struct TrimmableTrailingContent {
TrimmableTrailingContent(RunList&);
- void addFullyTrimmableContent(size_t runIndex, InlineLayoutUnit trimmableWidth);
+ void addFullyTrimmableContent(size_t runIndex, InlineLayoutUnit trimmableContentOffset, InlineLayoutUnit trimmableWidth);
void addPartiallyTrimmableContent(size_t runIndex, InlineLayoutUnit trimmableWidth);
InlineLayoutUnit remove();
InlineLayoutUnit removePartiallyTrimmableContent();
@@ -201,6 +202,7 @@
RunList& m_runs;
std::optional<size_t> m_firstTrimmableRunIndex;
bool m_hasFullyTrimmableContent { false };
+ InlineLayoutUnit m_trimmableContentOffset { 0 };
InlineLayoutUnit m_fullyTrimmableWidth { 0 };
InlineLayoutUnit m_partiallyTrimmableWidth { 0 };
};
@@ -245,6 +247,7 @@
m_firstTrimmableRunIndex = { };
m_fullyTrimmableWidth = { };
m_partiallyTrimmableWidth = { };
+ m_trimmableContentOffset = { };
}
inline void Line::HangingTrailingContent::reset()
Modified: trunk/Source/WebCore/layout/formattingContexts/inline/text/TextUtil.cpp (288054 => 288055)
--- trunk/Source/WebCore/layout/formattingContexts/inline/text/TextUtil.cpp 2022-01-15 13:32:05 UTC (rev 288054)
+++ trunk/Source/WebCore/layout/formattingContexts/inline/text/TextUtil.cpp 2022-01-15 13:36:49 UTC (rev 288055)
@@ -57,7 +57,7 @@
return TextUtil::width(inlineTextItem.inlineTextBox(), fontCascade, from, to, contentLogicalLeft);
}
-InlineLayoutUnit TextUtil::width(const InlineTextBox& inlineTextBox, const FontCascade& fontCascade, unsigned from, unsigned to, InlineLayoutUnit contentLogicalLeft)
+InlineLayoutUnit TextUtil::width(const InlineTextBox& inlineTextBox, const FontCascade& fontCascade, unsigned from, unsigned to, InlineLayoutUnit contentLogicalLeft, UseTrailingWhitespaceMeasuringOptimization useTrailingWhitespaceMeasuringOptimization)
{
if (from == to)
return 0;
@@ -65,8 +65,11 @@
auto text = inlineTextBox.content();
ASSERT(to <= text.length());
auto hasKerningOrLigatures = fontCascade.enableKerning() || fontCascade.requiresShaping();
- auto measureWithEndSpace = hasKerningOrLigatures && to < text.length() && text[to] == ' ';
- if (measureWithEndSpace)
+ // The "non-whitespace" + "whitespace" pattern is very common for inline content and since most of the "non-whitespace" runs end up with
+ // their "whitespace" pair on the line (notable exception is when trailing whitespace is trimmed).
+ // Including the trailing whitespace here enables us to cut the number of text measures when placing content on the line.
+ auto extendedMeasuring = useTrailingWhitespaceMeasuringOptimization == UseTrailingWhitespaceMeasuringOptimization::Yes && hasKerningOrLigatures && to < text.length() && text[to] == space;
+ if (extendedMeasuring)
++to;
float width = 0;
if (inlineTextBox.canUseSimplifiedContentMeasuring())
@@ -79,12 +82,21 @@
width = fontCascade.width(run);
}
- if (measureWithEndSpace)
+ if (extendedMeasuring)
width -= (fontCascade.spaceWidth() + fontCascade.wordSpacing());
return std::isnan(width) ? 0.0f : std::isinf(width) ? maxInlineLayoutUnit() : width;
}
+InlineLayoutUnit TextUtil::trailingWhitespaceWidth(const InlineTextBox& inlineTextBox, const FontCascade& fontCascade, size_t startPosition, size_t endPosition)
+{
+ auto text = inlineTextBox.content();
+ ASSERT(endPosition > startPosition + 1);
+ ASSERT(text[endPosition - 1] == space);
+ return width(inlineTextBox, fontCascade, startPosition, endPosition, { }, TextUtil::UseTrailingWhitespaceMeasuringOptimization::Yes) -
+ width(inlineTextBox, fontCascade, startPosition, endPosition - 1, { }, TextUtil::UseTrailingWhitespaceMeasuringOptimization::No);
+}
+
template <typename TextIterator>
static void fallbackFontsForRunWithIterator(HashSet<const Font*>& fallbackFonts, const FontCascade& fontCascade, const TextRun& run, TextIterator& textIterator)
{
Modified: trunk/Source/WebCore/layout/formattingContexts/inline/text/TextUtil.h (288054 => 288055)
--- trunk/Source/WebCore/layout/formattingContexts/inline/text/TextUtil.h 2022-01-15 13:32:05 UTC (rev 288054)
+++ trunk/Source/WebCore/layout/formattingContexts/inline/text/TextUtil.h 2022-01-15 13:36:49 UTC (rev 288055)
@@ -46,8 +46,12 @@
public:
static InlineLayoutUnit width(const InlineTextItem&, const FontCascade&, InlineLayoutUnit contentLogicalLeft);
static InlineLayoutUnit width(const InlineTextItem&, const FontCascade&, unsigned from, unsigned to, InlineLayoutUnit contentLogicalLeft);
- static InlineLayoutUnit width(const InlineTextBox&, const FontCascade&, unsigned from, unsigned to, InlineLayoutUnit contentLogicalLeft);
+ enum class UseTrailingWhitespaceMeasuringOptimization : uint8_t { Yes, No };
+ static InlineLayoutUnit width(const InlineTextBox&, const FontCascade&, unsigned from, unsigned to, InlineLayoutUnit contentLogicalLeft, UseTrailingWhitespaceMeasuringOptimization = UseTrailingWhitespaceMeasuringOptimization::Yes);
+
+ static InlineLayoutUnit trailingWhitespaceWidth(const InlineTextBox&, const FontCascade&, size_t startPosition, size_t endPosition);
+
using FallbackFontList = HashSet<const Font*>;
static FallbackFontList fallbackFontsForRun(const Line::Run&, const RenderStyle&);
