Reviewed by Anders.
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Jul 2006 15:53:47 +0000 (15:53 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Jul 2006 15:53:47 +0000 (15:53 +0000)
        - fix http://bugzilla.opendarwin.org/show_bug.cgi?id=9833
          REGRESSION: Reproducible crash: RenderMenuList.cpp:58: failed assertion `!m_first'

        * manual-tests/empty-title-popup.html: Added.

        * rendering/RenderMenuList.h: Add createInnerBlock.
        * rendering/RenderMenuList.cpp:
        (WebCore::RenderMenuList::createInnerBlock): Factored out of addChild.
        (WebCore::RenderMenuList::addChild): Call createInnerBlock.
        (WebCore::RenderMenuList::setText): Changed parameter type.
        (WebCore::RenderMenuList::showPopup): Call createInnerBlock before calling
        the parent class's addChild.

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

WebCore/ChangeLog
WebCore/manual-tests/empty-title-popup.html [new file with mode: 0644]
WebCore/rendering/RenderMenuList.cpp
WebCore/rendering/RenderMenuList.h

index 784963fb1f374de685a968ec4975f6a1069290b9..b202a50bd907f928feb2616c1f317c59eb04c45c 100644 (file)
@@ -1,3 +1,20 @@
+2006-07-10  Darin Adler  <darin@apple.com>
+
+        Reviewed by Anders.
+
+        - fix http://bugzilla.opendarwin.org/show_bug.cgi?id=9833
+          REGRESSION: Reproducible crash: RenderMenuList.cpp:58: failed assertion `!m_first'
+
+        * manual-tests/empty-title-popup.html: Added.
+
+        * rendering/RenderMenuList.h: Add createInnerBlock.
+        * rendering/RenderMenuList.cpp:
+        (WebCore::RenderMenuList::createInnerBlock): Factored out of addChild.
+        (WebCore::RenderMenuList::addChild): Call createInnerBlock.
+        (WebCore::RenderMenuList::setText): Changed parameter type.
+        (WebCore::RenderMenuList::showPopup): Call createInnerBlock before calling
+        the parent class's addChild.
+
 2006-07-10  Rob Buis  <buis@kde.org>
 
         Reviewed by Maciej via IRC.
@@ -6,7 +23,7 @@
         as defined in the spec.  http://paste.lisp.org/display/22342
 
         * ksvg2/svg/SVGEllipseElement.cpp:
-        (WebCore::SVGEllipseElement::rx):  Changed LM_HEIGHT to LM_WIDTH.
+        (WebCore::SVGEllipseElement::rx): Changed LM_HEIGHT to LM_WIDTH.
         * ksvg2/svg/SVGLineElement.cpp:
         (SVGLineElement::x2): Ditto.
 
diff --git a/WebCore/manual-tests/empty-title-popup.html b/WebCore/manual-tests/empty-title-popup.html
new file mode 100644 (file)
index 0000000..d5be5b9
--- /dev/null
@@ -0,0 +1,3 @@
+<p>This demonstrates <a href="http://bugzilla.opendarwin.org/show_bug.cgi?id=9833">bug 9833</a>.</p>
+<p>Select the second item on the pop up.
+<select><option></option><option>Pick me!</option></select></p>
index 5f3dd22703faa61a278202c1c836f569adc0d328..94285a94b6618f145728e284da91fc4927571a6a 100644 (file)
 #include "RenderTheme.h"
 #include <math.h>
 
+using namespace std;
+
 namespace WebCore {
 
 using namespace HTMLNames;
-using namespace std;
 
 RenderMenuList::RenderMenuList(HTMLSelectElement* element)
     : RenderFlexibleBox(element)
@@ -51,15 +52,24 @@ RenderMenuList::RenderMenuList(HTMLSelectElement* element)
 {
 }
 
-void RenderMenuList::addChild(RenderObject* newChild, RenderObject* beforeChild)
+void RenderMenuList::createInnerBlock()
 {
-    if (!m_inner) {
-        // Create an anonymous block.
-        assert(!m_first);
-        m_inner = createAnonymousBlock();
-        m_inner->style()->setBoxFlex(1.0f);
-        RenderFlexibleBox::addChild(m_inner);
+    if (m_inner) {
+        ASSERT(firstChild() == m_inner);
+        ASSERT(!m_inner->nextSibling());
+        return;
     }
+
+    // Create an anonymous block.
+    ASSERT(!firstChild());
+    m_inner = createAnonymousBlock();
+    m_inner->style()->setBoxFlex(1.0f);
+    RenderFlexibleBox::addChild(m_inner);
+}
+
+void RenderMenuList::addChild(RenderObject* newChild, RenderObject* beforeChild)
+{
+    createInnerBlock();
     m_inner->addChild(newChild, beforeChild);
 }
 
@@ -81,7 +91,7 @@ void RenderMenuList::setStyle(RenderStyle* style)
     if (m_inner)
         m_inner->style()->setBoxFlex(1.0f);
     if (m_popupMenu) {
-        RenderStyle *newStyle = new (renderArena()) RenderStyle();
+        RenderStyle* newStyle = new (renderArena()) RenderStyle;
         newStyle->inheritFrom(style);
         m_popupMenu->setStyle(newStyle);
     }
@@ -120,7 +130,7 @@ void RenderMenuList::updateFromElement()
     }
 }
 
-void RenderMenuList::setText(String s)
+void RenderMenuList::setText(const String& s)
 {
     if (s.isEmpty()) {
         if (m_buttonText) {
@@ -194,9 +204,13 @@ void RenderMenuList::calcMinMaxWidth()
 void RenderMenuList::showPopup()
 {
     if (!m_popupMenu) {
-        RenderStyle *newStyle = new (renderArena()) RenderStyle();
-        newStyle->inheritFrom(style());
+        // Create m_inner here so it ends up as the first child.
+        // This is important because otherwise we might try to create m_inner
+        // inside the showPopup call and it would fail.
+        createInnerBlock();
         m_popupMenu = theme()->createPopupMenu(renderArena(), document());
+        RenderStyle* newStyle = new (renderArena()) RenderStyle;
+        newStyle->inheritFrom(style());
         m_popupMenu->setStyle(newStyle);
         RenderFlexibleBox::addChild(m_popupMenu);
     }
@@ -210,10 +224,8 @@ void RenderMenuList::layout()
     RenderFlexibleBox::layout();
 }
 
-
 void RenderMenuList::updateSelection()
 {
-
 }
 
 void RenderMenuList::valueChanged(unsigned index)
index df41f18879152e6cb781e774bc3db482ea7c0ada..31367a2e445dcb6bc589e146308a70fb64f97349 100644 (file)
@@ -62,8 +62,9 @@ public:
     void valueChanged(unsigned index);
     bool hasPopupMenu() { return m_popupMenu; }
 
-protected:
-    void setText(String);
+private:
+    void createInnerBlock();
+    void setText(const String&);
 
     RenderText* m_buttonText;
     RenderBlock* m_inner;