[webkit-changes] [114666] trunk

[commit-queue]

Title: [114666] trunk

Revision

114666

Author

commit-qu...@webkit.org

Date

2012-04-19 13:15:18 -0700 (Thu, 19 Apr 2012)

Log Message

Positioned children of ruby runs not handled correctly during layout
https://bugs.webkit.org/show_bug.cgi?id=84157

Patch by Ken Buchanan <ke...@chromium.org> on 2012-04-19
Reviewed by Julien Chaffraix.

Source/WebCore:

A positioned ruby text causes a problem because it is excluded from
normal layout by its parent and instead gets layout from
RenderRubyRun::layoutSpecialExcludedChild; however this means it gets
skipped over during the loop in RenderBlock::layoutBlockChildren,
which is where positioned RenderBlocks get added to the appropriate
positioned object lists. As a result, a dirty positioned ruby text
will not get layout again unless the RenderRubyRun also needs layout
which is not guaranteed.

This patch disqualifies ruby text elements from being positioned.
If it is necessary to support this in future, ruby text layout will
have to be modified to ensure the renderers are added to the
appropriate block lists.

* css/CSSStyleSelector.cpp:
(WebCore::CSSStyleSelector::adjustRenderStyle):
* rendering/RenderRubyRun.cpp:
(WebCore::RenderRubyRun::rubyText):

LayoutTests:

* fast/ruby/modify-positioned-ruby-text-crash-expected.txt: Added
* fast/ruby/modify-positioned-ruby-text-crash.html: Exercises the crash condition
in but 84157 relating to positioned ruby text. This is being added
in case support for positioned ruby text is added in future, so that
we can guard against regression.
* fast/ruby/floating-ruby-text-expected.png: Added
* fast/ruby/floating-ruby-text-expected.txt: Added
* fast/ruby/floating-ruby-text.html: Creates a positioned ruby text. The
position should be ignored.
* fast/ruby/positioned-ruby-text-expected.png: Added
* fast/ruby/positioned-ruby-text-expected.txt: Added
* fast/ruby/positioned-ruby-text.html: Creates a floating ruby text. The
float should be ignored.

Modified Paths

• trunk/LayoutTests/ChangeLog
• trunk/Source/WebCore/ChangeLog
• trunk/Source/WebCore/css/CSSStyleSelector.cpp
• trunk/Source/WebCore/rendering/RenderRubyRun.cpp

Added Paths

• trunk/LayoutTests/fast/ruby/floating-ruby-text-expected.png
• trunk/LayoutTests/fast/ruby/floating-ruby-text-expected.txt
• trunk/LayoutTests/fast/ruby/floating-ruby-text.html
• trunk/LayoutTests/fast/ruby/modify-positioned-ruby-text-crash-expected.txt
• trunk/LayoutTests/fast/ruby/modify-positioned-ruby-text-crash.html
• trunk/LayoutTests/fast/ruby/positioned-ruby-text-expected.png
• trunk/LayoutTests/fast/ruby/positioned-ruby-text-expected.txt
• trunk/LayoutTests/fast/ruby/positioned-ruby-text.html

Diff

Modified: trunk/LayoutTests/ChangeLog (114665 => 114666)

--- trunk/LayoutTests/ChangeLog	2012-04-19 19:12:42 UTC (rev 114665)
+++ trunk/LayoutTests/ChangeLog	2012-04-19 20:15:18 UTC (rev 114666)
@@ -1,3 +1,24 @@
+2012-04-19  Ken Buchanan  <ke...@chromium.org>
+
+        Positioned children of ruby runs not handled correctly during layout
+        https://bugs.webkit.org/show_bug.cgi?id=84157
+
+        Reviewed by Julien Chaffraix.
+
+        * fast/ruby/modify-positioned-ruby-text-crash-expected.txt: Added
+        * fast/ruby/modify-positioned-ruby-text-crash.html: Exercises the crash condition
+        in but 84157 relating to positioned ruby text. This is being added
+        in case support for positioned ruby text is added in future, so that
+        we can guard against regression.
+        * fast/ruby/floating-ruby-text-expected.png: Added
+        * fast/ruby/floating-ruby-text-expected.txt: Added
+        * fast/ruby/floating-ruby-text.html: Creates a positioned ruby text. The
+        position should be ignored.
+        * fast/ruby/positioned-ruby-text-expected.png: Added
+        * fast/ruby/positioned-ruby-text-expected.txt: Added
+        * fast/ruby/positioned-ruby-text.html: Creates a floating ruby text. The
+        float should be ignored.
+
 2012-04-19  vse...@chromium.org  <vse...@chromium.org>
 
         Web Inspector: No response body available for cached resource requests with error status codes.

Added: trunk/LayoutTests/fast/ruby/floating-ruby-text-expected.png (0 => 114666)

--- trunk/LayoutTests/fast/ruby/floating-ruby-text-expected.png	                        (rev 0)
+++ trunk/LayoutTests/fast/ruby/floating-ruby-text-expected.png	2012-04-19 20:15:18 UTC (rev 114666)
@@ -0,0 +1,6 @@
+\x89PNG
+
+
+IHDR X')tEXtchecksum853de00567d121bea0b7bece66a5d61c`7\xFF\xFB
+\xAAIDATx\x9C\xED\xD6\xC1	 \xC00u\xFF\x9D\xCF%
+\x82$\xF4\xD9=3\x80\xCEy\xF0\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X\xB1
d\xAD4\xD1ӅIEND\xAEB`\x82
\ No newline at end of file

Added: trunk/LayoutTests/fast/ruby/floating-ruby-text-expected.txt (0 => 114666)

--- trunk/LayoutTests/fast/ruby/floating-ruby-text-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/ruby/floating-ruby-text-expected.txt	2012-04-19 20:15:18 UTC (rev 114666)
@@ -0,0 +1,15 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x44
+  RenderBlock {HTML} at (0,0) size 800x44
+    RenderBody {BODY} at (8,8) size 784x28
+      RenderRuby (inline) {RUBY} at (0,0) size 784x10 [color=#FFFFFF]
+        RenderRubyRun (anonymous) at (0,5) size 784x20
+          RenderRubyText {RT} at (0,-5) size 784x5
+            RenderText {#text} at (372,0) size 40x5
+              text run at (372,0) width 40: "rubytext"
+          RenderRubyBase (anonymous) at (0,0) size 784x20
+            RenderText {#text} at (0,0) size 784x20
+              text run at (0,0) width 784: "Attempt to create a floating ruby text element. Float is not supported for"
+              text run at (0,10) width 784: "ruby text, which should be apparent from the resulting render tree."
+      RenderText {#text} at (0,0) size 0x0

Added: trunk/LayoutTests/fast/ruby/floating-ruby-text.html (0 => 114666)

--- trunk/LayoutTests/fast/ruby/floating-ruby-text.html	                        (rev 0)
+++ trunk/LayoutTests/fast/ruby/floating-ruby-text.html	2012-04-19 20:15:18 UTC (rev 114666)
@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+<html>
+<ruby style="font-family: ahem; font-size: 10px; -webkit-font-smoothing: none; color: white;">Attempt to create a floating ruby text element. Float is not supported for ruby text, which should be apparent from the resulting render tree.<rt style="float:left">rubytext</rt></ruby>
+</html>

Added: trunk/LayoutTests/fast/ruby/modify-positioned-ruby-text-crash-expected.txt (0 => 114666)

--- trunk/LayoutTests/fast/ruby/modify-positioned-ruby-text-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/ruby/modify-positioned-ruby-text-crash-expected.txt	2012-04-19 20:15:18 UTC (rev 114666)
@@ -0,0 +1 @@
+ PASS, if no assert or crash in debug

Added: trunk/LayoutTests/fast/ruby/modify-positioned-ruby-text-crash.html (0 => 114666)

--- trunk/LayoutTests/fast/ruby/modify-positioned-ruby-text-crash.html	                        (rev 0)
+++ trunk/LayoutTests/fast/ruby/modify-positioned-ruby-text-crash.html	2012-04-19 20:15:18 UTC (rev 114666)
@@ -0,0 +1,19 @@
+<script>
+window._onload_ = function() {
+    var oSelection=window.getSelection();
+    document.execCommand("SelectAll", false, false)
+    var oRange = oSelection.getRangeAt(0);
+    var oDocumentFragment = oRange.extractContents();
+
+    document.body.appendChild(document.createTextNode('PASS, if no assert or crash in debug'));
+
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+}
+</script>
+<style>
+*{word-break:inherit;text-rendering:optimizeLegibility;}
+.fixedNavDown{nav-down:#;position:fixed; }
+</style>
+
+<ruby><rt class="fixedNavDown">A</rt></ruby>

Added: trunk/LayoutTests/fast/ruby/positioned-ruby-text-expected.png (0 => 114666)

--- trunk/LayoutTests/fast/ruby/positioned-ruby-text-expected.png	                        (rev 0)
+++ trunk/LayoutTests/fast/ruby/positioned-ruby-text-expected.png	2012-04-19 20:15:18 UTC (rev 114666)
@@ -0,0 +1,6 @@
+\x89PNG
+
+
+IHDR X')tEXtchecksum853de00567d121bea0b7bece66a5d61c`7\xFF\xFB
+\xAAIDATx\x9C\xED\xD6\xC1	 \xC00u\xFF\x9D\xCF%
+\x82$\xF4\xD9=3\x80\xCEy\xF0\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X\xB1
d\xAD4\xD1ӅIEND\xAEB`\x82
\ No newline at end of file

Added: trunk/LayoutTests/fast/ruby/positioned-ruby-text-expected.txt (0 => 114666)

--- trunk/LayoutTests/fast/ruby/positioned-ruby-text-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/ruby/positioned-ruby-text-expected.txt	2012-04-19 20:15:18 UTC (rev 114666)
@@ -0,0 +1,16 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x54
+  RenderBlock {HTML} at (0,0) size 800x54
+    RenderBody {BODY} at (8,8) size 784x38
+      RenderRuby (inline) {RUBY} at (0,0) size 784x10 [color=#FFFFFF]
+        RenderRubyRun (anonymous) at (0,5) size 784x30
+          RenderRubyText {RT} at (0,-5) size 784x5
+            RenderText {#text} at (372,0) size 40x5
+              text run at (372,0) width 40: "rubytext"
+          RenderRubyBase (anonymous) at (0,0) size 784x30
+            RenderText {#text} at (0,0) size 784x30
+              text run at (0,0) width 784: "Attempt to create a positioned ruby text element. Non-static position is not"
+              text run at (0,10) width 784: "supported for ruby text, which should be apparent from the resulting render"
+              text run at (0,20) width 50: "tree."
+      RenderText {#text} at (0,0) size 0x0

Added: trunk/LayoutTests/fast/ruby/positioned-ruby-text.html (0 => 114666)

--- trunk/LayoutTests/fast/ruby/positioned-ruby-text.html	                        (rev 0)
+++ trunk/LayoutTests/fast/ruby/positioned-ruby-text.html	2012-04-19 20:15:18 UTC (rev 114666)
@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+<html>
+<ruby style="font-family: ahem; font-size: 10px; -webkit-font-smoothing: none; color: white;">Attempt to create a positioned ruby text element. Non-static position is not supported for ruby text, which should be apparent from the resulting render tree.<rt style="position: fixed">rubytext</rt></ruby>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (114665 => 114666)

--- trunk/Source/WebCore/ChangeLog	2012-04-19 19:12:42 UTC (rev 114665)
+++ trunk/Source/WebCore/ChangeLog	2012-04-19 20:15:18 UTC (rev 114666)
@@ -1,3 +1,29 @@
+2012-04-19  Ken Buchanan  <ke...@chromium.org>
+
+        Positioned children of ruby runs not handled correctly during layout
+        https://bugs.webkit.org/show_bug.cgi?id=84157
+
+        Reviewed by Julien Chaffraix.
+
+        A positioned ruby text causes a problem because it is excluded from
+        normal layout by its parent and instead gets layout from
+        RenderRubyRun::layoutSpecialExcludedChild; however this means it gets
+        skipped over during the loop in RenderBlock::layoutBlockChildren,
+        which is where positioned RenderBlocks get added to the appropriate
+        positioned object lists. As a result, a dirty positioned ruby text
+        will not get layout again unless the RenderRubyRun also needs layout
+        which is not guaranteed.
+
+        This patch disqualifies ruby text elements from being positioned.
+        If it is necessary to support this in future, ruby text layout will
+        have to be modified to ensure the renderers are added to the
+        appropriate block lists.
+
+        * css/CSSStyleSelector.cpp:
+        (WebCore::CSSStyleSelector::adjustRenderStyle):
+        * rendering/RenderRubyRun.cpp:
+        (WebCore::RenderRubyRun::rubyText):
+
 2012-04-19  vse...@chromium.org  <vse...@chromium.org>
 
         Web Inspector: No response body available for cached resource requests with error status codes.

Modified: trunk/Source/WebCore/css/CSSStyleSelector.cpp (114665 => 114666)

--- trunk/Source/WebCore/css/CSSStyleSelector.cpp	2012-04-19 19:12:42 UTC (rev 114665)
+++ trunk/Source/WebCore/css/CSSStyleSelector.cpp	2012-04-19 20:15:18 UTC (rev 114666)
@@ -1954,6 +1954,12 @@
             style->setDisplay(BLOCK);
         }
 
+        // Ruby text does not support float or position. This might change with evolution of the specification.
+        if (e && e->hasTagName(rtTag)) {
+            style->setPosition(StaticPosition);
+            style->setFloating(NoFloat);
+        }
+
         // Table headers with a text-align of auto will change the text-align to center.
         if (e && e->hasTagName(thTag) && style->textAlign() == TAAUTO)
             style->setTextAlign(CENTER);

Modified: trunk/Source/WebCore/rendering/RenderRubyRun.cpp (114665 => 114666)

--- trunk/Source/WebCore/rendering/RenderRubyRun.cpp	2012-04-19 19:12:42 UTC (rev 114665)
+++ trunk/Source/WebCore/rendering/RenderRubyRun.cpp	2012-04-19 20:15:18 UTC (rev 114666)
@@ -74,6 +74,9 @@
 RenderRubyText* RenderRubyRun::rubyText() const
 {
     RenderObject* child = firstChild();
+    // If in future it becomes necessary to support floating or positioned ruby text,
+    // layout will have to be changed to handle them properly.
+    ASSERT(!child || !child->isRubyText() || !child->isFloatingOrPositioned());
     return child && child->isRubyText() ? static_cast<RenderRubyText*>(child) : 0;
 }
 

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes