Skip to content

Commit

Permalink
[IFC][Ruby] Line spacing is not even for contents that has ruby
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=271295
<rdar://122436686>

Reviewed by Antti Koivisto.

Let annotation spill into previous/next line's half-leading to keep vertical (horizontal) rhythm (i.e. do not stretch lines with ruby too much even if it means overlapping adjacent line boxes).

* LayoutTests/fast/ruby/tight-line-spacing-with-line-height-expected.html: Added.
* LayoutTests/fast/ruby/tight-line-spacing-with-line-height.html: Added.
* Source/WebCore/layout/formattingContexts/inline/ruby/RubyFormattingContext.cpp:
(WebCore::Layout::RubyFormattingContext::adjustLayoutBoundsAndStretchAncestorRubyBase):

Canonical link: https://commits.webkit.org/276402@main
  • Loading branch information
alanbaradlay committed Mar 20, 2024
1 parent bcddb3e commit 983282c
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<style>
div {
padding-top: 5px;
padding-bottom: 5px;
line-height: 16px;
font-size: 16px;
font-family: Ahem;
width: 150px;
outline: 1px solid blue;
}
rt {
font-size: 16px;
}
</style>
<div>X<br>X<br>X<br></div>

<pre>PASS if annotation tightly fits.</pre>
15 changes: 15 additions & 0 deletions LayoutTests/fast/ruby/tight-line-spacing-with-line-height.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<style>
div {
line-height: 26px;
font-size: 16px;
font-family: Ahem;
width: 150px;
outline: 1px solid blue;
}
rt {
font-size: 16px;
}
</style>
<div>X<br><ruby>X<rt>x</rt></ruby></div>

<pre>PASS if annotation tightly fits.</pre>
1 change: 1 addition & 0 deletions LayoutTests/platform/ios/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -4676,6 +4676,7 @@ imported/w3c/web-platform-tests/css/css-ruby/ruby-text-combine-upright-002a.html
imported/w3c/web-platform-tests/css/css-ruby/ruby-text-combine-upright-002b.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/css-ruby/line-break-around-ruby-001.html [ Failure ]
fast/ruby/annotation-with-line-gap.html [ ImageOnlyFailure ]
fast/ruby/tight-line-spacing-with-line-height.html [ ImageOnlyFailure ]

webkit.org/b/264001 [ Debug ] imported/w3c/web-platform-tests/requestidlecallback/callback-multiple-calls.html [ Pass Failure ]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,8 @@ void RubyFormattingContext::adjustLayoutBoundsAndStretchAncestorRubyBase(LineBox
auto over = InlineLayoutUnit { };
auto under = InlineLayoutUnit { };
auto annotationBoxLogicalHeight = InlineLayoutUnit { inlineFormattingContext.geometryForBox(*annotationBox).marginBoxHeight() };
if (rubyPosition(rubyBaseLayoutBox) == RubyPosition::Before)
auto isAnnotationBefore = rubyPosition(rubyBaseLayoutBox) == RubyPosition::Before;
if (isAnnotationBefore)
over = annotationBoxLogicalHeight;
else
under = annotationBoxLogicalHeight;
Expand Down Expand Up @@ -453,8 +454,13 @@ void RubyFormattingContext::adjustLayoutBoundsAndStretchAncestorRubyBase(LineBox
layoutBounds.descent = std::max(descentWithAnnotation, layoutBounds.descent);
} else if (layoutBounds.height() < ascentWithAnnotation + descentWithAnnotation) {
// In case line-height does not produce enough space for annotation.
layoutBounds.ascent = std::max(ascentWithAnnotation, layoutBounds.ascent);
layoutBounds.descent = std::max(descentWithAnnotation, layoutBounds.descent);
auto extraSpaceNeededForAnnotation = (ascentWithAnnotation + descentWithAnnotation) - layoutBounds.height();
// Note that this makes annotation leak into previous/next line's (bottom/top) half leading. It ensures though that we don't
// overly stretch lines and break (logical) vertical rhythm too much.
if (isAnnotationBefore)
layoutBounds.ascent += extraSpaceNeededForAnnotation;
else
layoutBounds.descent += extraSpaceNeededForAnnotation;
}
}

Expand Down

0 comments on commit 983282c

Please sign in to comment.