Justified ruby can cause lines to grow beyond their container
https://bugs.webkit.org/show_bug.cgi?id=141732
Reviewed by David Hyatt.
Source/WebCore:
After we re-layout RenderRubyRuns, this can change the environment upon which
ruby's overhang calculation is sensitive to. Before this patch, we would recalculate
the overhang after the RenderRubyRun gets relaid out. However, doing such causes the
effective width of the RenderRubyRun to change, which causes out subsequent
justification calculations to be off.
Therefore, we have a cycle; the amount of ruby overhang can change the justification
in a line, and the layout of the line affects the ruby overhang calculation. Instead
of performing a layout in a loop until it converges, this patch simply observes that
having a flush right edge is more valuable than having a perfectly correct overhang.
It therefore simply removes the secondary overhang calculation.
Test: fast/text/ruby-justification-flush.html
* rendering/RenderBlockFlow.h:
* rendering/RenderBlockLineLayout.cpp:
(WebCore::RenderBlockFlow::updateRubyForJustifiedText):
(WebCore::RenderBlockFlow::computeExpansionForJustifiedText):
(WebCore::RenderBlockFlow::computeInlineDirectionPositionsForSegment):
LayoutTests:
Make sure that the right edge of a justified ruby line matches up with
the same line without ruby.
* fast/text/ruby-justification-flush-expected.html: Added.
* fast/text/ruby-justification-flush.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@180278 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 773b0be..946c642 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2015-02-18 Myles C. Maxfield <mmaxfield@apple.com>
+
+ Justified ruby can cause lines to grow beyond their container
+ https://bugs.webkit.org/show_bug.cgi?id=141732
+
+ Reviewed by David Hyatt.
+
+ Make sure that the right edge of a justified ruby line matches up with
+ the same line without ruby.
+
+ * fast/text/ruby-justification-flush-expected.html: Added.
+ * fast/text/ruby-justification-flush.html: Added.
+
2015-02-18 Eric Carlson <eric.carlson@apple.com>
[iOS] pause video when a tab moves to the background on some devices
diff --git a/LayoutTests/fast/text/ruby-justification-flush-expected.html b/LayoutTests/fast/text/ruby-justification-flush-expected.html
new file mode 100644
index 0000000..d5b644d
--- /dev/null
+++ b/LayoutTests/fast/text/ruby-justification-flush-expected.html
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<style>
+#outer {
+ position: absolute;
+ width: 500px;
+ height: 500px;
+ overflow: hidden;
+}
+
+#inner {
+ font-size: 127px;
+ text-align: justify;
+ position: absolute;
+ bottom: 0px;
+ left: 73px;
+ width: 1200px;
+ font-family: Ahem;
+}
+</style>
+</head>
+<body>
+This test makes sure that ruby overhangs don't make text grow beyond the bound of the enclosing box.
+At the bottom left of this page, there are two black squares on top of each other. This test passes if the two squares are exactly vertically aligned.
+<div id="outer"><div id="inner">a<br>a<br> </div></div>
+</body>
+</html>
diff --git a/LayoutTests/fast/text/ruby-justification-flush.html b/LayoutTests/fast/text/ruby-justification-flush.html
new file mode 100644
index 0000000..957fcf2
--- /dev/null
+++ b/LayoutTests/fast/text/ruby-justification-flush.html
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<style>
+#outer {
+ position: absolute;
+ width: 500px;
+ height: 500px;
+ overflow: hidden;
+}
+
+#inner {
+ font-size: 127px;
+ text-align: justify;
+ position: absolute;
+ bottom: 0px;
+ left: -1000px;
+ width: 1200px;
+ font-family: Ahem;
+}
+</style>
+</head>
+<body>
+This test makes sure that ruby overhangs don't make text grow beyond the bound of the enclosing box.
+At the bottom left of this page, there are two black squares on top of each other. This test passes if the two squares are exactly vertically aligned.
+<div id="outer"><div id="inner">a<ruby> a <rt>aaaa</rt> a <rt>aaaa</rt></ruby>a a a a a a a a</div></div>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 0b45f75..9fa2f98 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,30 @@
+2015-02-18 Myles C. Maxfield <mmaxfield@apple.com>
+
+ Justified ruby can cause lines to grow beyond their container
+ https://bugs.webkit.org/show_bug.cgi?id=141732
+
+ Reviewed by David Hyatt.
+
+ After we re-layout RenderRubyRuns, this can change the environment upon which
+ ruby's overhang calculation is sensitive to. Before this patch, we would recalculate
+ the overhang after the RenderRubyRun gets relaid out. However, doing such causes the
+ effective width of the RenderRubyRun to change, which causes out subsequent
+ justification calculations to be off.
+
+ Therefore, we have a cycle; the amount of ruby overhang can change the justification
+ in a line, and the layout of the line affects the ruby overhang calculation. Instead
+ of performing a layout in a loop until it converges, this patch simply observes that
+ having a flush right edge is more valuable than having a perfectly correct overhang.
+ It therefore simply removes the secondary overhang calculation.
+
+ Test: fast/text/ruby-justification-flush.html
+
+ * rendering/RenderBlockFlow.h:
+ * rendering/RenderBlockLineLayout.cpp:
+ (WebCore::RenderBlockFlow::updateRubyForJustifiedText):
+ (WebCore::RenderBlockFlow::computeExpansionForJustifiedText):
+ (WebCore::RenderBlockFlow::computeInlineDirectionPositionsForSegment):
+
2015-02-18 Chris Dumez <cdumez@apple.com>
Evict dead resources in MemoryCache in MemoryPressureHandler::releaseNoncriticalMemory()
diff --git a/Source/WebCore/rendering/RenderBlockFlow.h b/Source/WebCore/rendering/RenderBlockFlow.h
index 63971ae..2bf602a 100644
--- a/Source/WebCore/rendering/RenderBlockFlow.h
+++ b/Source/WebCore/rendering/RenderBlockFlow.h
@@ -555,8 +555,8 @@
RootInlineBox* constructLine(BidiRunList<BidiRun>&, const LineInfo&);
void setMarginsForRubyRun(BidiRun*, RenderRubyRun&, RenderObject*, const LineInfo&);
void computeInlineDirectionPositionsForLine(RootInlineBox*, const LineInfo&, BidiRun* firstRun, BidiRun* trailingSpaceRun, bool reachedEnd, GlyphOverflowAndFallbackFontsMap&, VerticalPositionCache&, WordMeasurements&);
- void updateRubyForJustifiedText(RenderRubyRun&, BidiRun&, const Vector<unsigned, 16>& expansionOpportunities, unsigned& expansionOpportunityCount, float& totalLogicalWidth, float availableLogicalWidth, RenderObject* previousObject, const LineInfo&, size_t& expansionIndex);
- void computeExpansionForJustifiedText(BidiRun* firstRun, BidiRun* trailingSpaceRun, const Vector<unsigned, 16>& expansionOpportunities, unsigned expansionOpportunityCount, float totalLogicalWidth, float availableLogicalWidth, const LineInfo&);
+ void updateRubyForJustifiedText(RenderRubyRun&, BidiRun&, const Vector<unsigned, 16>& expansionOpportunities, unsigned& expansionOpportunityCount, float& totalLogicalWidth, float availableLogicalWidth, size_t& expansionIndex);
+ void computeExpansionForJustifiedText(BidiRun* firstRun, BidiRun* trailingSpaceRun, const Vector<unsigned, 16>& expansionOpportunities, unsigned expansionOpportunityCount, float totalLogicalWidth, float availableLogicalWidth);
BidiRun* computeInlineDirectionPositionsForSegment(RootInlineBox*, const LineInfo&, ETextAlign, float& logicalLeft,
float& availableLogicalWidth, BidiRun* firstRun, BidiRun* trailingSpaceRun, GlyphOverflowAndFallbackFontsMap& textBoxDataMap, VerticalPositionCache&, WordMeasurements&);
void computeBlockDirectionPositionsForLine(RootInlineBox*, BidiRun*, GlyphOverflowAndFallbackFontsMap&, VerticalPositionCache&);
diff --git a/Source/WebCore/rendering/RenderBlockLineLayout.cpp b/Source/WebCore/rendering/RenderBlockLineLayout.cpp
index 6896e27..99d9fa3 100644
--- a/Source/WebCore/rendering/RenderBlockLineLayout.cpp
+++ b/Source/WebCore/rendering/RenderBlockLineLayout.cpp
@@ -545,7 +545,7 @@
}
}
-void RenderBlockFlow::updateRubyForJustifiedText(RenderRubyRun& rubyRun, BidiRun& r, const Vector<unsigned, 16>& expansionOpportunities, unsigned& expansionOpportunityCount, float& totalLogicalWidth, float availableLogicalWidth, RenderObject* previousObject, const LineInfo& lineInfo, size_t& i)
+void RenderBlockFlow::updateRubyForJustifiedText(RenderRubyRun& rubyRun, BidiRun& r, const Vector<unsigned, 16>& expansionOpportunities, unsigned& expansionOpportunityCount, float& totalLogicalWidth, float availableLogicalWidth, size_t& i)
{
if (!rubyRun.rubyBase() || !rubyRun.rubyBase()->firstRootBox() || rubyRun.rubyBase()->firstRootBox()->nextRootBox() || !r.renderer().style().collapseWhiteSpace())
return;
@@ -580,7 +580,6 @@
}
rubyRun.layoutBlock(true);
rubyRun.clearOverrideLogicalContentWidth();
- setMarginsForRubyRun(&r, rubyRun, previousObject, lineInfo); // Expanding the base might mean there's less of a need for overhang
r.box()->setExpansion(newRubyRunWidth - r.box()->logicalWidth());
// This relayout caused the size of the RenderRubyText and the RenderRubyBase to change, dependent on the line's current expansion. Next time we relayout the
@@ -596,19 +595,15 @@
}
}
-void RenderBlockFlow::computeExpansionForJustifiedText(BidiRun* firstRun, BidiRun* trailingSpaceRun, const Vector<unsigned, 16>& expansionOpportunities, unsigned expansionOpportunityCount, float totalLogicalWidth, float availableLogicalWidth, const LineInfo& lineInfo)
+void RenderBlockFlow::computeExpansionForJustifiedText(BidiRun* firstRun, BidiRun* trailingSpaceRun, const Vector<unsigned, 16>& expansionOpportunities, unsigned expansionOpportunityCount, float totalLogicalWidth, float availableLogicalWidth)
{
if (!expansionOpportunityCount || availableLogicalWidth <= totalLogicalWidth)
return;
- RenderObject* previousObject = nullptr;
-
size_t i = 0;
for (BidiRun* run = firstRun; run; run = run->next()) {
- if (!run->box() || run == trailingSpaceRun) {
- previousObject = &run->renderer();
+ if (!run->box() || run == trailingSpaceRun)
continue;
- }
if (is<RenderText>(run->renderer())) {
unsigned opportunitiesInRun = expansionOpportunities[i++];
@@ -624,9 +619,7 @@
}
expansionOpportunityCount -= opportunitiesInRun;
} else if (is<RenderRubyRun>(run->renderer()))
- updateRubyForJustifiedText(downcast<RenderRubyRun>(run->renderer()), *run, expansionOpportunities, expansionOpportunityCount, totalLogicalWidth, availableLogicalWidth, previousObject, lineInfo, i);
-
- previousObject = &run->renderer();
+ updateRubyForJustifiedText(downcast<RenderRubyRun>(run->renderer()), *run, expansionOpportunities, expansionOpportunityCount, totalLogicalWidth, availableLogicalWidth, i);
if (!expansionOpportunityCount)
break;
@@ -813,7 +806,7 @@
updateLogicalWidthForAlignment(textAlign, lineBox, trailingSpaceRun, logicalLeft, totalLogicalWidth, availableLogicalWidth, expansionOpportunityCount);
- computeExpansionForJustifiedText(firstRun, trailingSpaceRun, expansionOpportunities, expansionOpportunityCount, totalLogicalWidth, availableLogicalWidth, lineInfo);
+ computeExpansionForJustifiedText(firstRun, trailingSpaceRun, expansionOpportunities, expansionOpportunityCount, totalLogicalWidth, availableLogicalWidth);
return run;
}
