2011-04-13 Roland Steiner <rolandsteiner@chromium.org>
authorrolandsteiner@chromium.org <rolandsteiner@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Apr 2011 23:52:15 +0000 (23:52 +0000)
committerrolandsteiner@chromium.org <rolandsteiner@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Apr 2011 23:52:15 +0000 (23:52 +0000)
        Reviewed by David Hyatt.

        Bug 55930 - Incorrect handling of 'display:' property within nested <ruby> tags
        https://bugs.webkit.org/show_bug.cgi?id=55930

        Test that 'display: block' and 'display: table' on generated :before/:after content
        within <ruby> doesn't crash.

        * fast/ruby/after-block-doesnt-crash-expected.txt: Added.
        * fast/ruby/after-block-doesnt-crash.html: Added.
        * fast/ruby/after-table-doesnt-crash-expected.txt: Added.
        * fast/ruby/after-table-doesnt-crash.html: Added.
        * fast/ruby/before-block-doesnt-crash-expected.txt: Added.
        * fast/ruby/before-block-doesnt-crash.html: Added.
        * fast/ruby/before-table-doesnt-crash-expected.txt: Added.
        * fast/ruby/before-table-doesnt-crash.html: Added.
2011-04-13  Roland Steiner  <rolandsteiner@chromium.org>

        Reviewed by David Hyatt.

        Bug 55930 - Incorrect handling of 'display:' property within nested <ruby> tags
        https://bugs.webkit.org/show_bug.cgi?id=55930

        Non-inline :before/:after generated content is now wrapped with an anonymous inline block.

        Also, added an additional check in RenderObjectChildList::updateBeforeAfterContent()
        to verify that the created render object is legal under the parent.

        Tests: fast/ruby/after-block-doesnt-crash.html
               fast/ruby/after-table-doesnt-crash.html
               fast/ruby/before-block-doesnt-crash.html
               fast/ruby/before-table-doesnt-crash.html

        * rendering/RenderObjectChildList.cpp:
        (WebCore::RenderObjectChildList::updateBeforeAfterContent):
        * rendering/RenderRuby.cpp:
        (WebCore::isAnonymousRubyInlineBlock):
        (WebCore::rubyBeforeBlock):
        (WebCore::rubyAfterBlock):
        (WebCore::createAnonymousRubyInlineBlock):
        (WebCore::lastRubyRun):
        (WebCore::RenderRubyAsInline::addChild):
        (WebCore::RenderRubyAsInline::removeChild):
        (WebCore::RenderRubyAsBlock::addChild):
        (WebCore::RenderRubyAsBlock::removeChild):
        * rendering/RenderRuby.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@83787 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/ruby/after-block-doesnt-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/ruby/after-block-doesnt-crash.html [new file with mode: 0644]
LayoutTests/fast/ruby/after-table-doesnt-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/ruby/after-table-doesnt-crash.html [new file with mode: 0644]
LayoutTests/fast/ruby/before-block-doesnt-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/ruby/before-block-doesnt-crash.html [new file with mode: 0644]
LayoutTests/fast/ruby/before-table-doesnt-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/ruby/before-table-doesnt-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderObjectChildList.cpp
Source/WebCore/rendering/RenderRuby.cpp
Source/WebCore/rendering/RenderRuby.h

index 0e69b5e..181fc28 100644 (file)
@@ -1,3 +1,22 @@
+2011-04-13  Roland Steiner  <rolandsteiner@chromium.org>
+
+        Reviewed by David Hyatt.
+
+        Bug 55930 - Incorrect handling of 'display:' property within nested <ruby> tags
+        https://bugs.webkit.org/show_bug.cgi?id=55930
+
+        Test that 'display: block' and 'display: table' on generated :before/:after content
+        within <ruby> doesn't crash.
+        
+        * fast/ruby/after-block-doesnt-crash-expected.txt: Added.
+        * fast/ruby/after-block-doesnt-crash.html: Added.
+        * fast/ruby/after-table-doesnt-crash-expected.txt: Added.
+        * fast/ruby/after-table-doesnt-crash.html: Added.
+        * fast/ruby/before-block-doesnt-crash-expected.txt: Added.
+        * fast/ruby/before-block-doesnt-crash.html: Added.
+        * fast/ruby/before-table-doesnt-crash-expected.txt: Added.
+        * fast/ruby/before-table-doesnt-crash.html: Added.
+
 2011-04-13  Matthew Delaney  <mdelaney@apple.com>
 
         Reviewed by Chris Marrin.
diff --git a/LayoutTests/fast/ruby/after-block-doesnt-crash-expected.txt b/LayoutTests/fast/ruby/after-block-doesnt-crash-expected.txt
new file mode 100644 (file)
index 0000000..ed36d69
--- /dev/null
@@ -0,0 +1,2 @@
+Blocked access to external URL http://YY/
+This test passes if it doesn't crash.
diff --git a/LayoutTests/fast/ruby/after-block-doesnt-crash.html b/LayoutTests/fast/ruby/after-block-doesnt-crash.html
new file mode 100644 (file)
index 0000000..d3dd27c
--- /dev/null
@@ -0,0 +1,23 @@
+<meta http-equiv="refresh" content="1;url=" />
+<style>
+    ruby::after {
+        display: block;
+        content: url("http://YY");
+    }
+</style>
+<ruby>
+    <ruby>
+        <ruby>
+            <style>
+                ruby {
+                    float: right;      
+                }
+            </style>
+        </ruby>
+    </ruby>
+</ruby>
+<script>
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+</script>
+This test passes if it doesn't crash.
diff --git a/LayoutTests/fast/ruby/after-table-doesnt-crash-expected.txt b/LayoutTests/fast/ruby/after-table-doesnt-crash-expected.txt
new file mode 100644 (file)
index 0000000..ed36d69
--- /dev/null
@@ -0,0 +1,2 @@
+Blocked access to external URL http://YY/
+This test passes if it doesn't crash.
diff --git a/LayoutTests/fast/ruby/after-table-doesnt-crash.html b/LayoutTests/fast/ruby/after-table-doesnt-crash.html
new file mode 100644 (file)
index 0000000..a474506
--- /dev/null
@@ -0,0 +1,23 @@
+<meta http-equiv="refresh" content="1;url=" />
+<style>
+    ruby::after {
+        display: table;
+        content: url("http://YY");
+    }
+</style>
+<ruby>
+    <ruby>
+        <ruby>
+            <style>
+                ruby {
+                    float: right;      
+                }
+            </style>
+        </ruby>
+    </ruby>
+</ruby>
+<script>
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+</script>
+This test passes if it doesn't crash.
diff --git a/LayoutTests/fast/ruby/before-block-doesnt-crash-expected.txt b/LayoutTests/fast/ruby/before-block-doesnt-crash-expected.txt
new file mode 100644 (file)
index 0000000..b8aa09a
--- /dev/null
@@ -0,0 +1,2 @@
+Blocked access to external URL http://XX/
+This test passes if it doesn't crash.
diff --git a/LayoutTests/fast/ruby/before-block-doesnt-crash.html b/LayoutTests/fast/ruby/before-block-doesnt-crash.html
new file mode 100644 (file)
index 0000000..2451a98
--- /dev/null
@@ -0,0 +1,23 @@
+<meta http-equiv="refresh" content="1;url=" />
+<style>
+    ruby::before {
+        display: block;
+        content: url("http://XX");
+    }
+</style>
+<ruby>
+    <ruby>
+        <ruby>
+            <style>
+                ruby {
+                    float: right;      
+                }
+            </style>
+        </ruby>
+    </ruby>
+</ruby>
+<script>
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+</script>
+This test passes if it doesn't crash.
diff --git a/LayoutTests/fast/ruby/before-table-doesnt-crash-expected.txt b/LayoutTests/fast/ruby/before-table-doesnt-crash-expected.txt
new file mode 100644 (file)
index 0000000..b8aa09a
--- /dev/null
@@ -0,0 +1,2 @@
+Blocked access to external URL http://XX/
+This test passes if it doesn't crash.
diff --git a/LayoutTests/fast/ruby/before-table-doesnt-crash.html b/LayoutTests/fast/ruby/before-table-doesnt-crash.html
new file mode 100644 (file)
index 0000000..2451a98
--- /dev/null
@@ -0,0 +1,23 @@
+<meta http-equiv="refresh" content="1;url=" />
+<style>
+    ruby::before {
+        display: block;
+        content: url("http://XX");
+    }
+</style>
+<ruby>
+    <ruby>
+        <ruby>
+            <style>
+                ruby {
+                    float: right;      
+                }
+            </style>
+        </ruby>
+    </ruby>
+</ruby>
+<script>
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+</script>
+This test passes if it doesn't crash.
index 5a626c4..a2df5a8 100644 (file)
@@ -1,3 +1,34 @@
+2011-04-13  Roland Steiner  <rolandsteiner@chromium.org>
+
+        Reviewed by David Hyatt.
+
+        Bug 55930 - Incorrect handling of 'display:' property within nested <ruby> tags
+        https://bugs.webkit.org/show_bug.cgi?id=55930
+
+        Non-inline :before/:after generated content is now wrapped with an anonymous inline block.
+
+        Also, added an additional check in RenderObjectChildList::updateBeforeAfterContent()
+        to verify that the created render object is legal under the parent.
+
+        Tests: fast/ruby/after-block-doesnt-crash.html
+               fast/ruby/after-table-doesnt-crash.html
+               fast/ruby/before-block-doesnt-crash.html
+               fast/ruby/before-table-doesnt-crash.html
+
+        * rendering/RenderObjectChildList.cpp:
+        (WebCore::RenderObjectChildList::updateBeforeAfterContent):
+        * rendering/RenderRuby.cpp:
+        (WebCore::isAnonymousRubyInlineBlock):
+        (WebCore::rubyBeforeBlock):
+        (WebCore::rubyAfterBlock):
+        (WebCore::createAnonymousRubyInlineBlock):
+        (WebCore::lastRubyRun):
+        (WebCore::RenderRubyAsInline::addChild):
+        (WebCore::RenderRubyAsInline::removeChild):
+        (WebCore::RenderRubyAsBlock::addChild):
+        (WebCore::RenderRubyAsBlock::removeChild):
+        * rendering/RenderRuby.h:
+
 2011-04-13  Matthew Delaney  <mdelaney@apple.com>
 
         Reviewed by Simon Fraser.
index 617067a..e26f221 100644 (file)
@@ -456,6 +456,12 @@ void RenderObjectChildList::updateBeforeAfterContent(RenderObject* owner, Pseudo
                 ASSERT(styledObject->node()); // The styled object cannot be anonymous or else it could not have ':before' or ':after' pseudo elements.
                 generatedContentContainer->setNode(styledObject->node()); // This allows access to the generatingNode.
                 generatedContentContainer->setStyle(pseudoElementStyle);
+                if (!owner->isChildAllowed(generatedContentContainer, pseudoElementStyle)) {
+                    // The generated content container is not allowed here -> abort.
+                    generatedContentContainer->destroy();
+                    renderer->destroy();
+                    return;
+                }
                 owner->addChild(generatedContentContainer, insertBefore);
             }
             if (generatedContentContainer->isChildAllowed(renderer, pseudoElementStyle))
index 1c5cfaf..0b51384 100644 (file)
 #include "RenderRuby.h"
 
 #include "RenderRubyRun.h"
+#include "RenderStyle.h"
+#include <wtf/RefPtr.h>
 
 namespace WebCore {
 
 //=== generic helper functions to avoid excessive code duplication ===
 
+static inline bool isAnonymousRubyInlineBlock(RenderObject* object)
+{
+    ASSERT(!object->parent()->isRuby()
+        || object->isRubyRun()
+        || (object->isInline() && (object->isBeforeContent() || object->isAfterContent()))
+        || (object->isAnonymous() && object->isRenderBlock() && object->style()->display() == INLINE_BLOCK));
+    return object->parent()->isRuby() && object->isRenderBlock() && !object->isRubyRun();
+}
+
+static inline RenderBlock* rubyBeforeBlock(const RenderObject* ruby)
+{
+    RenderObject* child = ruby->firstChild();
+    return child && !child->isRubyRun() && child->isRenderBlock() && child->style()->styleType() == BEFORE ? static_cast<RenderBlock*>(child) : 0;
+}
+
+static inline RenderBlock* rubyAfterBlock(const RenderObject* ruby)
+{
+    RenderObject* child = ruby->lastChild();
+    return child && !child->isRubyRun() && child->isRenderBlock() && child->style()->styleType() == AFTER ? static_cast<RenderBlock*>(child) : 0;
+}
+
+static RenderBlock* createAnonymousRubyInlineBlock(RenderObject* ruby, PseudoId styleType)
+{
+    RefPtr<RenderStyle> newStyle = RenderStyle::createAnonymousStyle(ruby->style());
+    newStyle->setDisplay(INLINE_BLOCK);
+    newStyle->setStyleType(styleType);
+    
+    RenderBlock* newBlock = new (ruby->renderArena()) RenderBlock(ruby->document() /* anonymous box */);
+    newBlock->setStyle(newStyle.release());
+    return newBlock;
+}
+
 static RenderRubyRun* lastRubyRun(const RenderObject* ruby)
 {
     RenderObject* child = ruby->lastChild();
-    if (child && ruby->isAfterContent(child))
+    if (child && !child->isRubyRun())
         child = child->previousSibling();
-    ASSERT(!child || child->isRubyRun() || child->isBeforeContent());
+    ASSERT(!child || child->isRubyRun() || child->isBeforeContent() || child == rubyBeforeBlock(ruby));
     return child && child->isRubyRun() ? static_cast<RenderRubyRun*>(child) : 0;
 }
 
@@ -65,18 +99,37 @@ RenderRubyAsInline::~RenderRubyAsInline()
 {
 }
 
-bool RenderRubyAsInline::isChildAllowed(RenderObject* child, RenderStyle*) const
-{
-    return child->isRubyText()
-        || child->isRubyRun()
-        || child->isInline();
-}
-
 void RenderRubyAsInline::addChild(RenderObject* child, RenderObject* beforeChild)
 {
-    // Insert :before and :after content outside of ruby runs.
-    if (child->isBeforeContent() || child->isAfterContent()) {
-        RenderInline::addChild(child, beforeChild);
+    // Insert :before and :after content before/after the RenderRubyRun(s)
+    if (child->isBeforeContent()) {
+        if (child->isInline()) {
+            // Add generated inline content normally
+            RenderInline::addChild(child, firstChild());
+        } else {
+            // Wrap non-inline content with an anonymous inline-block.
+            RenderBlock* beforeBlock = rubyBeforeBlock(this);
+            if (!beforeBlock) {
+                beforeBlock = createAnonymousRubyInlineBlock(this, BEFORE);
+                RenderInline::addChild(beforeBlock, firstChild());
+            }
+            beforeBlock->addChild(child);
+        }
+        return;
+    }
+    if (child->isAfterContent()) {
+        if (child->isInline()) {
+            // Add generated inline content normally
+            RenderInline::addChild(child);
+        } else {
+            // Wrap non-inline content with an anonymous inline-block.
+            RenderBlock* afterBlock = rubyAfterBlock(this);
+            if (!afterBlock) {
+                afterBlock = createAnonymousRubyInlineBlock(this, AFTER);
+                RenderInline::addChild(afterBlock);
+            }
+            afterBlock->addChild(child);
+        }
         return;
     }
 
@@ -113,15 +166,23 @@ void RenderRubyAsInline::addChild(RenderObject* child, RenderObject* beforeChild
 
 void RenderRubyAsInline::removeChild(RenderObject* child)
 {
-    // If the child's parent is *this (a ruby run or :before or :after content),
+    // If the child's parent is *this (must be a ruby run or generated content or anonymous block),
     // just use the normal remove method.
-    if (child->isRubyRun() || child->isBeforeContent() || child->isAfterContent()) {
+    if (child->parent() == this) {
+        ASSERT(child->isRubyRun() || child->isBeforeContent() || child->isAfterContent() || isAnonymousRubyInlineBlock(child));
         RenderInline::removeChild(child);
         return;
     }
+    // If the child's parent is an anoymous block (must be generated :before/:after content)
+    // just use the block's remove method.
+    if (isAnonymousRubyInlineBlock(child->parent())) {
+        ASSERT(child->isBeforeContent() || child->isAfterContent());
+        child->parent()->removeChild(child);
+        removeChild(child->parent());
+        return;
+    }
 
     // Otherwise find the containing run and remove it from there.
-    ASSERT(child->parent() != this);
     RenderRubyRun* run = findRubyRunParent(child);
     ASSERT(run);
     run->removeChild(child);
@@ -139,18 +200,37 @@ RenderRubyAsBlock::~RenderRubyAsBlock()
 {
 }
 
-bool RenderRubyAsBlock::isChildAllowed(RenderObject* child, RenderStyle*) const
-{
-    return child->isRubyText()
-        || child->isRubyRun()
-        || child->isInline();
-}
-
 void RenderRubyAsBlock::addChild(RenderObject* child, RenderObject* beforeChild)
 {
-    // Insert :before and :after content outside of ruby runs.
-    if (child->isBeforeContent() || child->isAfterContent()) {
-        RenderBlock::addChild(child, beforeChild);
+    // Insert :before and :after content before/after the RenderRubyRun(s)
+    if (child->isBeforeContent()) {
+        if (child->isInline()) {
+            // Add generated inline content normally
+            RenderBlock::addChild(child, firstChild());
+        } else {
+            // Wrap non-inline content with an anonymous inline-block.
+            RenderBlock* beforeBlock = rubyBeforeBlock(this);
+            if (!beforeBlock) {
+                beforeBlock = createAnonymousRubyInlineBlock(this, BEFORE);
+                RenderBlock::addChild(beforeBlock, firstChild());
+            }
+            beforeBlock->addChild(child);
+        }
+        return;
+    }
+    if (child->isAfterContent()) {
+        if (child->isInline()) {
+            // Add generated inline content normally
+            RenderBlock::addChild(child);
+        } else {
+            // Wrap non-inline content with an anonymous inline-block.
+            RenderBlock* afterBlock = rubyAfterBlock(this);
+            if (!afterBlock) {
+                afterBlock = createAnonymousRubyInlineBlock(this, AFTER);
+                RenderBlock::addChild(afterBlock);
+            }
+            afterBlock->addChild(child);
+        }
         return;
     }
 
@@ -187,15 +267,23 @@ void RenderRubyAsBlock::addChild(RenderObject* child, RenderObject* beforeChild)
 
 void RenderRubyAsBlock::removeChild(RenderObject* child)
 {
-    // If the child's parent is *this (a ruby run or :before or :after content),
+    // If the child's parent is *this (must be a ruby run or generated content or anonymous block),
     // just use the normal remove method.
-    if (child->isRubyRun() || child->isBeforeContent() || child->isAfterContent()) {
+    if (child->parent() == this) {
+        ASSERT(child->isRubyRun() || child->isBeforeContent() || child->isAfterContent() || isAnonymousRubyInlineBlock(child));
         RenderBlock::removeChild(child);
         return;
     }
+    // If the child's parent is an anoymous block (must be generated :before/:after content)
+    // just use the block's remove method.
+    if (isAnonymousRubyInlineBlock(child->parent())) {
+        ASSERT(child->isBeforeContent() || child->isAfterContent());
+        child->parent()->removeChild(child);
+        removeChild(child->parent());
+        return;
+    }
 
     // Otherwise find the containing run and remove it from there.
-    ASSERT(child->parent() != this);
     RenderRubyRun* run = findRubyRunParent(child);
     ASSERT(run);
     run->removeChild(child);
index 49a84d8..24ac0db 100644 (file)
@@ -47,6 +47,8 @@ namespace WebCore {
 //              1-n inline object(s)
 //
 // Note: <rp> elements are defined as having 'display:none' and thus normally are not assigned a renderer.
+//
+// Generated :before/:after content is shunted into anonymous inline blocks
 
 // <ruby> when used as 'display:inline'
 class RenderRubyAsInline : public RenderInline {
@@ -54,7 +56,6 @@ public:
     RenderRubyAsInline(Node*);
     virtual ~RenderRubyAsInline();
 
-    virtual bool isChildAllowed(RenderObject*, RenderStyle*) const;
     virtual void addChild(RenderObject* child, RenderObject* beforeChild = 0);
     virtual void removeChild(RenderObject* child);
 
@@ -71,7 +72,6 @@ public:
     RenderRubyAsBlock(Node*);
     virtual ~RenderRubyAsBlock();
 
-    virtual bool isChildAllowed(RenderObject*, RenderStyle*) const;
     virtual void addChild(RenderObject* child, RenderObject* beforeChild = 0);
     virtual void removeChild(RenderObject* child);