Reviewed and tweaked by Darin.
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 5 Feb 2006 00:49:09 +0000 (00:49 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 5 Feb 2006 00:49:09 +0000 (00:49 +0000)
        - fixed regressions caused by fix for bug 5776 earlier today

        * rendering/render_list.h: Remove m_value from RenderListMarker, add it to RenderListItem.
        A few other tweaks, including getting rid of "friend" relationship.
        * rendering/render_list.cpp:
        (RenderListItem::RenderListItem): Initialize m_value.
        (RenderListItem::setStyle): Restore old behavior of making no marker for LNONE case.
        (RenderListItem::calcValue): Work on m_value, not m_marker->m_value.
        (RenderListItem::resetValue): Reset m_value, even if there's no marker.
        (RenderListMarker::RenderListMarker): Remove code to set up m_value.
        (RenderListMarker::calcMinMaxWidth): Get marker value from list item.

        * rendering/RenderContainer.cpp: (updateListMarkerNumbers): Call resetValue by its new name.

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

WebCore/ChangeLog
WebCore/rendering/RenderContainer.cpp
WebCore/rendering/render_list.cpp
WebCore/rendering/render_list.h

index 13729e3a6dd9abf710bfab34bea3a01209c8cd85..41fdf7ff4a1f833a01b3f360dfbe7adf25e3fdce 100644 (file)
@@ -1,3 +1,21 @@
+2006-02-04  Andrew Wellington  <proton@wiretapped.net>
+
+        Reviewed and tweaked by Darin.
+
+        - fixed regressions caused by fix for bug 5776 earlier today
+
+        * rendering/render_list.h: Remove m_value from RenderListMarker, add it to RenderListItem.
+        A few other tweaks, including getting rid of "friend" relationship.
+        * rendering/render_list.cpp:
+        (RenderListItem::RenderListItem): Initialize m_value.
+        (RenderListItem::setStyle): Restore old behavior of making no marker for LNONE case.
+        (RenderListItem::calcValue): Work on m_value, not m_marker->m_value.
+        (RenderListItem::resetValue): Reset m_value, even if there's no marker.
+        (RenderListMarker::RenderListMarker): Remove code to set up m_value.
+        (RenderListMarker::calcMinMaxWidth): Get marker value from list item.
+
+        * rendering/RenderContainer.cpp: (updateListMarkerNumbers): Call resetValue by its new name.
+
 2006-02-04  Maciej Stachowiak  <mjs@apple.com>
 
         Reviewed by Hyatt.
index 383109c7edc580b36a082ddcd00e0a967cba44ef..57793271a035f9bd59e29d8dca9e752b287376bc 100644 (file)
@@ -82,16 +82,11 @@ bool RenderContainer::canHaveChildren() const
 static void updateListMarkerNumbers(RenderObject *child)
 {
     for (RenderObject *r = child; r && r->isListItem(); r = r->nextSibling())
-        static_cast<RenderListItem *>(r)->resetMarkerValue();
+        static_cast<RenderListItem *>(r)->resetValue();
 }
 
 void RenderContainer::addChild(RenderObject *newChild, RenderObject *beforeChild)
 {
-#ifdef DEBUG_LAYOUT
-    kdDebug( 6040 ) << this << ": " <<  renderName() << "(RenderObject)::addChild( " << newChild << ": " <<
-        newChild->renderName() << ", " << (beforeChild ? beforeChild->renderName() : "0") << " )" << endl;
-#endif
-
     bool needsTable = false;
 
     if(!newChild->isText() && !newChild->isReplaced()) {
@@ -115,18 +110,14 @@ void RenderContainer::addChild(RenderObject *newChild, RenderObject *beforeChild
         case TABLE_ROW_GROUP:
         case TABLE_HEADER_GROUP:
         case TABLE_FOOTER_GROUP:
-
-            //kdDebug( 6040 ) << "adding section" << endl;
             if ( !isTable() )
                 needsTable = true;
             break;
         case TABLE_ROW:
-            //kdDebug( 6040 ) << "adding row" << endl;
             if ( !isTableSection() )
                 needsTable = true;
             break;
         case TABLE_CELL:
-            //kdDebug( 6040 ) << "adding cell" << endl;
             if ( !isTableRow() )
                 needsTable = true;
             // I'm not 100% sure this is the best way to fix this, but without this
@@ -148,7 +139,6 @@ void RenderContainer::addChild(RenderObject *newChild, RenderObject *beforeChild
         if( beforeChild && beforeChild->isAnonymous() && beforeChild->isTable() )
             table = static_cast<RenderTable *>(beforeChild);
         else {
-            //kdDebug( 6040 ) << "creating anonymous table" << endl;
             table = new (renderArena()) RenderTable(document() /* is anonymous */);
             RenderStyle *newStyle = new (renderArena()) RenderStyle();
             newStyle->inheritFrom(style());
index 4a42de49eb728f7589a21d255cccee56b1a55b64..48599093e17942cbb6818653bed6d643261861fc 100644 (file)
@@ -131,20 +131,18 @@ static QString toHebrew( int number ) {
 // -------------------------------------------------------------------------
 
 RenderListItem::RenderListItem(DOM::NodeImpl* node)
-    : RenderBlock(node), _notInList(false)
+    : RenderBlock(node), predefVal(-1), m_marker(0), _notInList(false), m_value(-1)
 {
     // init RenderObject attributes
     setInline(false);   // our object is not Inline
-
-    predefVal = -1;
-    m_marker = 0;
 }
 
 void RenderListItem::setStyle(RenderStyle *_style)
 {
     RenderBlock::setStyle(_style);
 
-    if (!(style()->listStyleImage() && style()->listStyleImage()->isErrorImage())) {
+    if (style()->listStyleType() != LNONE ||
+        (style()->listStyleImage() && !style()->listStyleImage()->isErrorImage())) {
         RenderStyle *newStyle = new (renderArena()) RenderStyle();
         newStyle->ref();
         // The marker always inherits from the list item, regardless of where it might end
@@ -163,10 +161,6 @@ void RenderListItem::setStyle(RenderStyle *_style)
     }
 }
 
-RenderListItem::~RenderListItem()
-{
-}
-
 void RenderListItem::destroy()
 {    
     if (m_marker) {
@@ -203,28 +197,24 @@ static RenderListItem* previousListItem(NodeImpl* list, RenderListItem* item)
     return 0;
 }
 
-void RenderListItem::calcListValue()
+void RenderListItem::calcValue()
 {
-    // Called by the marker, so we know there is a marker.
-    ASSERT(m_marker);
-
-    // Only called when the value is known.
-    ASSERT(m_marker->m_value == -1);
-
-    int value;
     if (predefVal != -1)
-        value = predefVal;
+        m_value = predefVal;
     else {
         NodeImpl* list = enclosingList(node());
         RenderListItem* item = previousListItem(list, this);
-        if (item)
-            value = item->value() + 1;
-        else if (list && list->hasTagName(olTag))
-            value = static_cast<HTMLOListElementImpl*>(list)->start();
+        if (item) {
+            // FIXME: This recurses to a possible depth of the length of the list.
+            // That's not good -- we need to change this to an iterative algorithm.
+            if (item->value() == -1)
+                item->calcValue();
+            m_value = item->value() + 1;
+        } else if (list && list->hasTagName(olTag))
+            m_value = static_cast<HTMLOListElementImpl*>(list)->start();
         else
-            value = 1;
+            m_value = 1;
     }
-    m_marker->m_value = value;
 }
 
 bool RenderListItem::isEmpty() const
@@ -263,13 +253,11 @@ static RenderObject* getParentOfFirstLineBox(RenderObject* curr, RenderObject* m
     return 0;
 }
 
-void RenderListItem::resetMarkerValue()
+void RenderListItem::resetValue()
 {
-    if (!m_marker)
-        return;
-
-    m_marker->m_value = -1;
-    m_marker->setNeedsLayoutAndMinMaxRecalc();
+    m_value = -1;
+    if (m_marker)
+        m_marker->setNeedsLayoutAndMinMaxRecalc();
 }
 
 void RenderListItem::updateMarkerLocation()
@@ -314,7 +302,7 @@ void RenderListItem::layout( )
 {
     KHTMLAssert( needsLayout() );
     KHTMLAssert( minMaxKnown() );
-
+    
     updateMarkerLocation();    
     RenderBlock::layout();
 }
@@ -358,13 +346,11 @@ IntRect RenderListItem::getAbsoluteRepaintRect()
 // -----------------------------------------------------------
 
 RenderListMarker::RenderListMarker(DocumentImpl* document)
-    : RenderBox(document), m_listImage(0), m_value(-1)
+    : RenderBox(document), m_listImage(0)
 {
     // init RenderObject attributes
     setInline(true);   // our object is Inline
     setReplaced(true); // pretend to be replaced
-    // val = -1;
-    // m_listImage = 0;
 }
 
 RenderListMarker::~RenderListMarker()
@@ -573,8 +559,8 @@ void RenderListMarker::calcMinMaxWidth()
         return;
     }
 
-    if (m_value < 0) // not yet calculated
-        m_listItem->calcListValue();
+    if (m_listItem->value() < 0) // not yet calculated
+        m_listItem->calcValue();
 
     const QFontMetrics &fm = style()->fontMetrics();
     m_height = fm.ascent();
@@ -597,17 +583,17 @@ void RenderListMarker::calcMinMaxWidth()
     case DECIMAL_LEADING_ZERO:
         // ### unsupported, we use decimal instead
     case LDECIMAL:
-        m_item.sprintf( "%d", m_value );
+        m_item.sprintf( "%d", m_listItem->value() );
         break;
     case LOWER_ROMAN:
-        m_item = toRoman( m_value, false );
+        m_item = toRoman( m_listItem->value(), false );
         break;
     case UPPER_ROMAN:
-        m_item = toRoman( m_value, true );
+        m_item = toRoman( m_listItem->value(), true );
         break;
     case LOWER_GREEK:
      {
-       int number = m_value - 1;
+       int number = m_listItem->value() - 1;
        int l = (number % 24);
 
        if (l>16) {l++;} // Skip GREEK SMALL LETTER FINAL SIGMA
@@ -619,15 +605,15 @@ void RenderListMarker::calcMinMaxWidth()
        break;
      }
     case HEBREW:
-       m_item = toHebrew( m_value );
+       m_item = toHebrew( m_listItem->value() );
        break;
     case LOWER_ALPHA:
     case LOWER_LATIN:
-        m_item = toLetter( m_value, 'a' );
+        m_item = toLetter( m_listItem->value(), 'a' );
         break;
     case UPPER_ALPHA:
     case UPPER_LATIN:
-        m_item = toLetter( m_value, 'A' );
+        m_item = toLetter( m_listItem->value(), 'A' );
         break;
     case LNONE:
         break;
index c7e47902d568a9220e71881f5697264ac91f915f..eb2d7f907fba04b854ecfe26303869556f1db9ec 100644 (file)
@@ -3,7 +3,7 @@
  *
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
- * Copyright (C) 2003 Apple Computer, Inc.
+ * Copyright (C) 2003, 2004, 2005, 2006 Apple Computer, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -28,8 +28,7 @@
 
 // ### list-style-position, list-style-image is still missing
 
-namespace khtml
-{
+namespace WebCore {
 
 class RenderListItem;
 
@@ -70,14 +69,11 @@ public:
     
     const QString& text() const { return m_item; }
 
-protected:
-    friend class RenderListItem;
-    
     bool isInside() const;
 
+private:
     QString m_item;
     CachedImage *m_listImage;
-    int m_value;
     RenderListItem* m_listItem;
 };
 
@@ -92,7 +88,6 @@ class RenderListItem : public RenderBlock
 {
 public:
     RenderListItem(DOM::NodeImpl*);
-    virtual ~RenderListItem();
     
     virtual void destroy();
 
@@ -102,10 +97,11 @@ public:
 
     virtual bool isListItem() const { return true; }
     
-    int value() const { return m_marker->m_value; }
-    void setValue( int v ) { predefVal = v; }
-    void calcListValue();
-    
+    int value() const { return m_value; }
+    void setValue(int v) { predefVal = v; }
+    void calcValue();
+    void resetValue();
+
     virtual bool isEmpty() const;
     virtual void paint(PaintInfo& i, int xoff, int yoff);
 
@@ -119,15 +115,15 @@ public:
     void setNotInList(bool notInList) { _notInList = notInList; }
     bool notInList() const { return _notInList; }
 
-    void resetMarkerValue();
-    QString markerStringValue() { if (m_marker) return m_marker->m_item; return ""; }
+    QString markerStringValue() { return m_marker ? m_marker->text() : ""; }
 
-protected:
+private:
     int predefVal;
     RenderListMarker *m_marker;
     bool _notInList;
+    int m_value;
 };
 
-}; //namespace
+} //namespace
 
 #endif