Justified ruby can cause lines to grow beyond their container
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Feb 2015 19:39:17 +0000 (19:39 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Feb 2015 19:39:17 +0000 (19:39 +0000)
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: https://svn.webkit.org/repository/webkit/trunk@180278 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/text/ruby-justification-flush-expected.html [new file with mode: 0644]
LayoutTests/fast/text/ruby-justification-flush.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlockFlow.h
Source/WebCore/rendering/RenderBlockLineLayout.cpp

index 773b0beb985035c6c6cc1f421de07ed6339b1343..946c642c6ff766699670fedad7c545c7709279bc 100644 (file)
@@ -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 (file)
index 0000000..d5b644d
--- /dev/null
@@ -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>&nbsp;</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 (file)
index 0000000..957fcf2
--- /dev/null
@@ -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>
index 0b45f75a7675b30a50c8f1a358cae32277ba2d41..9fa2f981d59f3b98296ef8eadd5d4926e592ab04 100644 (file)
@@ -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()
index 63971ae26ce91faed3c69ff3ac79ede6f3ee9d1c..2bf602a27ee9dbdea1f8ab6c9a7cbad12c43e13b 100644 (file)
@@ -555,8 +555,8 @@ private:
     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&);
index 6896e273b6cbf0743aa0f731c56fd96bb7434c9f..99d9fa3afc1d374b871d082320f6ea72055bf64d 100644 (file)
@@ -545,7 +545,7 @@ static inline void setLogicalWidthForTextRun(RootInlineBox* lineBox, BidiRun* ru
     }
 }
 
-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 @@ void RenderBlockFlow::updateRubyForJustifiedText(RenderRubyRun& rubyRun, BidiRun
         }
         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::updateRubyForJustifiedText(RenderRubyRun& rubyRun, BidiRun
     }
 }
 
-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 @@ void RenderBlockFlow::computeExpansionForJustifiedText(BidiRun* firstRun, BidiRu
             }
             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 @@ BidiRun* RenderBlockFlow::computeInlineDirectionPositionsForSegment(RootInlineBo
 
     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;
 }