Skip to content

Commit

Permalink
Using ruby tag within a hanging-punctuation specifies element, hangin…
Browse files Browse the repository at this point in the history
…g appears not to apply.

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

Reviewed by Antti Koivisto.

Let's take punctuation offset into account when processing (left-to-right) bidi based inline content.

1. Pass leading punctuation width over to display content build as part of the line layout result
2. Offset "visual left start position" with the punctuation width (negative offset)
- non-bidi content works fine as we already take this negative offset into account while building the line -which we lose when dealing with bidi reordering
- right-to-left is also fine as punctuation width being visually trailing does not contribute to visual left

* LayoutTests/fast/text/hanging-punctuation-with-bidi-expected.html: Added.
* LayoutTests/fast/text/hanging-punctuation-with-bidi.html: Added.
* Source/WebCore/layout/formattingContexts/inline/InlineLine.cpp:
(WebCore::Layout::Line::close):
* Source/WebCore/layout/formattingContexts/inline/InlineLine.h:
(WebCore::Layout::Line::HangingContent::leadingPunctuationWidth const):
* Source/WebCore/layout/formattingContexts/inline/InlineLineBuilder.cpp:
(WebCore::Layout::LineBuilder::layoutInlineContent):
* Source/WebCore/layout/formattingContexts/inline/LineLayoutResult.h:
* Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.cpp:
(WebCore::Layout::InlineDisplayContentBuilder::processBidiContent):

Canonical link: https://commits.webkit.org/276026@main
  • Loading branch information
alanbaradlay committed Mar 13, 2024
1 parent 99e7a28 commit 722de1a
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 2 deletions.
15 changes: 15 additions & 0 deletions LayoutTests/fast/text/hanging-punctuation-with-bidi-expected.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<meta charset="utf-8">
<style>
div {
font-family: Ahem;
font-size: 20px;
background-color: blue;
hanging-punctuation: first;
color: green;
margin-left: 12px;
}
</style>
<div>“content</div>
<div>“content</div>
<div>“content</div>
<div>“content</div>
15 changes: 15 additions & 0 deletions LayoutTests/fast/text/hanging-punctuation-with-bidi.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<meta charset="utf-8">
<style>
div {
font-family: Ahem;
font-size: 20px;
background-color: blue;
hanging-punctuation: first;
color: green;
margin-left: 12px;
}
</style>
<div>“content</div>
<div><span>content</span></div>
<div><span style="unicode-bidi: isolate">content</span></div>
<div><ruby>content</ruby></div>
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ Line::Result Line::close()
, contentLogicalRight
, !!m_hangingContent.trailingWhitespaceLength()
, m_hangingContent.trailingWidth()
, m_hangingContent.leadingPunctuationWidth()
, m_hasNonDefaultBidiLevelRun
, m_nonSpanningInlineLevelBoxCount
};
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/layout/formattingContexts/inline/InlineLine.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ class Line {
InlineLayoutUnit contentLogicalRight { 0.f };
bool isHangingTrailingContentWhitespace { false };
InlineLayoutUnit hangingTrailingContentWidth { 0.f };
InlineLayoutUnit hangablePunctuationStartWidth { 0.f };
bool contentNeedsBidiReordering { false };
size_t nonSpanningInlineLevelBoxCount { 0 };
};
Expand Down Expand Up @@ -269,6 +270,7 @@ class Line {
InlineLayoutUnit trailingWidth() const { return m_trailingContent ? m_trailingContent->width : 0.f; }
InlineLayoutUnit trailingWhitespaceWidth() const { return m_trailingContent && m_trailingContent->type == TrailingContent::Type::Whitespace ? m_trailingContent->width : 0.f; }

InlineLayoutUnit leadingPunctuationWidth() const { return m_leadingPunctuationWidth; }
InlineLayoutUnit width() const { return m_leadingPunctuationWidth + trailingWidth(); }

size_t length() const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ LineLayoutResult LineBuilder::layoutInlineContent(const LineInput& lineInput, co
, { WTFMove(m_placedFloats), WTFMove(m_suspendedFloats), m_lineIsConstrainedByFloat }
, { contentLogicalLeft, result.contentLogicalWidth, contentLogicalLeft + result.contentLogicalRight, lineContent.overflowLogicalWidth }
, { m_lineLogicalRect.topLeft(), m_lineLogicalRect.width(), m_lineInitialLogicalRect.left() + m_initialIntrusiveFloatsWidth, m_initialLetterClearGap }
, { !result.isHangingTrailingContentWhitespace, result.hangingTrailingContentWidth }
, { !result.isHangingTrailingContentWhitespace, result.hangingTrailingContentWidth, result.hangablePunctuationStartWidth }
, { WTFMove(visualOrderList), inlineBaseDirection }
, { isFirstFormattedLine() ? LineLayoutResult::IsFirstLast::FirstFormattedLine::WithinIFC : LineLayoutResult::IsFirstLast::FirstFormattedLine::No, isLastInlineContent }
, { WTFMove(lineContent.rubyBaseAlignmentOffsetList), lineContent.rubyAnnotationOffset }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ struct LineLayoutResult {
struct HangingContent {
bool shouldContributeToScrollableOverflow { false };
InlineLayoutUnit logicalWidth { 0.f };
InlineLayoutUnit hangablePunctuationStartWidth { 0.f };
};
HangingContent hangingContent { };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,8 @@ void InlineDisplayContentBuilder::processBidiContent(const LineLayoutResult& lin

auto lineLogicalTop = isHorizontalWritingMode ? m_displayLine.top() : m_displayLine.left();
auto lineLogicalLeft = isHorizontalWritingMode ? m_displayLine.left() : m_displayLine.top();
auto contentStartInInlineDirectionVisualOrder = lineLogicalLeft + m_displayLine.contentLogicalLeftIgnoringInlineDirection();
// Note that hangable punctuation does not contribute to contentLogicalLeftIgnoringInlineDirection() as it is considered more of an overflow.
auto contentStartInInlineDirectionVisualOrder = lineLogicalLeft + m_displayLine.contentLogicalLeftIgnoringInlineDirection() - (rootStyle().isLeftToRightDirection() ? lineLayoutResult.hangingContent.hangablePunctuationStartWidth : 0.f);
auto hasInlineBox = false;
auto createDisplayBoxesInVisualOrder = [&] {

Expand Down

0 comments on commit 722de1a

Please sign in to comment.