Bug 1290813 - Correctly number indirect descendants of <ol reversed>; r?xidorn draft
authorManish Goregaokar <manishearth@gmail.com>
Mon, 01 Aug 2016 15:54:25 +0530
changeset 394912 83f6be13619ffeb50253888c691b488150b9bf30
parent 394782 4a18b5cacb1b21a3e8b4b1dada6b2dd3dba51cb1
child 394913 8815cc4d955c4e356f503b0c6d5e6a93f4a45717
child 394918 7c320e2d47bfe84032410654db743243dabf74c5
child 394931 6eae88b097c45f879597a110f143f8f9fc2aad99
push id24671
push usermanishearth@gmail.com
push dateMon, 01 Aug 2016 10:22:27 +0000
reviewersxidorn
bugs1290813
milestone50.0a1
Bug 1290813 - Correctly number indirect descendants of <ol reversed>; r?xidorn MozReview-Commit-ID: 6HYtCrgdK13
layout/generic/nsBlockFrame.cpp
layout/generic/nsBlockFrame.h
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -7012,56 +7012,51 @@ nsBlockFrame::RenumberLists(nsPresContex
   MOZ_ASSERT(hc, "How is mContent not HTML?");
   const nsAttrValue* attr = hc->GetParsedAttr(nsGkAtoms::start);
   if (attr && attr->Type() == nsAttrValue::eInteger) {
     ordinal = attr->GetIntegerValue();
   } else if (increment < 0) {
     // <ol reversed> case, or some other case with a negative increment: count
     // up the child list
     ordinal = 0;
-    for (nsIContent* kid = mContent->GetFirstChild(); kid;
-         kid = kid->GetNextSibling()) {
-      if (kid->IsHTMLElement(nsGkAtoms::li)) {
-        // FIXME: This isn't right in terms of what CSS says to do for
-        // overflow of counters (but it only matters when this node has
-        // more than numeric_limits<int32_t>::max() children).
-        ordinal -= increment;
-      }
-    }
+    nsBlockFrame* block = static_cast<nsBlockFrame*>(FirstInFlow());
+    RenumberListsInBlock(aPresContext, block, &ordinal, 0, -increment, true);
   }
 
   // Get to first-in-flow
   nsBlockFrame* block = static_cast<nsBlockFrame*>(FirstInFlow());
-  return RenumberListsInBlock(aPresContext, block, &ordinal, 0, increment);
+  return RenumberListsInBlock(aPresContext, block, &ordinal, 0, increment, false);
 }
 
 bool
 nsBlockFrame::RenumberListsInBlock(nsPresContext* aPresContext,
                                    nsBlockFrame* aBlockFrame,
                                    int32_t* aOrdinal,
                                    int32_t aDepth,
-                                   int32_t aIncrement)
+                                   int32_t aIncrement,
+                                   bool aForCounting)
 {
   // Examine each line in the block
   bool foundValidLine;
   nsBlockInFlowLineIterator bifLineIter(aBlockFrame, &foundValidLine);
   
   if (!foundValidLine)
     return false;
 
   bool renumberedABullet = false;
 
   do {
     nsLineList::iterator line = bifLineIter.GetLine();
     nsIFrame* kid = line->mFirstChild;
     int32_t n = line->GetChildCount();
     while (--n >= 0) {
       bool kidRenumberedABullet = RenumberListsFor(aPresContext, kid, aOrdinal,
-                                                   aDepth, aIncrement);
-      if (kidRenumberedABullet) {
+                                                   aDepth, aIncrement,
+                                                   aForCounting);
+      if (!aForCounting && kidRenumberedABullet) {
         line->MarkDirty();
         renumberedABullet = true;
       }
       kid = kid->GetNextSibling();
     }
   } while (bifLineIter.Next());
 
   // We need to set NS_FRAME_HAS_DIRTY_CHILDREN bits up the tree between
@@ -7076,17 +7071,18 @@ nsBlockFrame::RenumberListsInBlock(nsPre
   return renumberedABullet;
 }
 
 bool
 nsBlockFrame::RenumberListsFor(nsPresContext* aPresContext,
                                nsIFrame* aKid,
                                int32_t* aOrdinal,
                                int32_t aDepth,
-                               int32_t aIncrement)
+                               int32_t aIncrement,
+                               bool aForCounting)
 {
   NS_PRECONDITION(aPresContext && aKid && aOrdinal, "null params are immoral!");
 
   // add in a sanity check for absurdly deep frame trees.  See bug 42138
   if (MAX_DEPTH_FOR_LIST_RENUMBERING < aDepth)
     return false;
 
   // if the frame is a placeholder, then get the out of flow frame
@@ -7116,41 +7112,50 @@ nsBlockFrame::RenumberListsFor(nsPresCon
   // ordinal.
   if (NS_STYLE_DISPLAY_LIST_ITEM == display->mDisplay) {
     // Make certain that the frame is a block frame in case
     // something foreign has crept in.
     nsBlockFrame* listItem = nsLayoutUtils::GetAsBlock(kid);
     if (listItem) {
       nsBulletFrame* bullet = listItem->GetBullet();
       if (bullet) {
-        bool changed;
-        *aOrdinal = bullet->SetListItemOrdinal(*aOrdinal, &changed, aIncrement);
-        if (changed) {
-          kidRenumberedABullet = true;
-
-          // The ordinal changed - mark the bullet frame, and any
-          // intermediate frames between it and the block (are there
-          // ever any?), dirty.
-          // The calling code will make the necessary FrameNeedsReflow
-          // call for the list ancestor.
-          bullet->AddStateBits(NS_FRAME_IS_DIRTY);
-          nsIFrame *f = bullet;
-          do {
-            nsIFrame *parent = f->GetParent();
-            parent->ChildIsDirty(f);
-            f = parent;
-          } while (f != listItem);
+        if (!aForCounting) {
+          bool changed;
+          *aOrdinal = bullet->SetListItemOrdinal(*aOrdinal, &changed, aIncrement);
+          if (changed) {
+            kidRenumberedABullet = true;
+
+            // The ordinal changed - mark the bullet frame, and any
+            // intermediate frames between it and the block (are there
+            // ever any?), dirty.
+            // The calling code will make the necessary FrameNeedsReflow
+            // call for the list ancestor.
+            bullet->AddStateBits(NS_FRAME_IS_DIRTY);
+            nsIFrame *f = bullet;
+            do {
+              nsIFrame *parent = f->GetParent();
+              parent->ChildIsDirty(f);
+              f = parent;
+            } while (f != listItem);
+          }
+        } else {
+          // We're only counting the number of children,
+          // not restyling them. Don't take |value|
+          // into account when incrementing the ordinal
+          // or dirty the bullet.
+          *aOrdinal += aIncrement;
         }
       }
 
       // XXX temporary? if the list-item has child list-items they
       // should be numbered too; especially since the list-item is
       // itself (ASSUMED!) not to be a counter-resetter.
       bool meToo = RenumberListsInBlock(aPresContext, listItem, aOrdinal,
-                                        aDepth + 1, aIncrement);
+                                        aDepth + 1, aIncrement,
+                                        aForCounting);
       if (meToo) {
         kidRenumberedABullet = true;
       }
     }
   }
   else if (NS_STYLE_DISPLAY_BLOCK == display->mDisplay) {
     if (FrameStartsCounterScope(kid)) {
       // Don't bother recursing into a block frame that is a new
@@ -7159,17 +7164,18 @@ nsBlockFrame::RenumberListsFor(nsPresCon
     }
     else {
       // If the display=block element is a block frame then go ahead
       // and recurse into it, as it might have child list-items.
       nsBlockFrame* kidBlock = nsLayoutUtils::GetAsBlock(kid);
       if (kidBlock) {
         kidRenumberedABullet = RenumberListsInBlock(aPresContext, kidBlock,
                                                     aOrdinal, aDepth + 1,
-                                                    aIncrement);
+                                                    aIncrement,
+                                                    aForCounting);
       }
     }
   }
   return kidRenumberedABullet;
 }
 
 void
 nsBlockFrame::ReflowBullet(nsIFrame* aBulletFrame,
--- a/layout/generic/nsBlockFrame.h
+++ b/layout/generic/nsBlockFrame.h
@@ -782,25 +782,54 @@ protected:
   // List handling kludge
 
   // If this returns true, the block it's called on should get the
   // NS_FRAME_HAS_DIRTY_CHILDREN bit set on it by the caller; either directly
   // if it's already in reflow, or via calling FrameNeedsReflow() to schedule a
   // reflow.
   bool RenumberLists(nsPresContext* aPresContext);
 
+  /**
+   * Renumber lists for a single block frame
+   * @param aOrdinal Ordinal number to start counting at.
+   *        Modifies this number for each associated list
+   *        item. Changes in the numbering due to setting
+   *        the |value| attribute are included if |aForCounting|
+   *        is false. This value is both an input and output
+   *        of this function, with the output value being the
+   *        next ordinal number to be used.
+   * @param aIncrement Amount to increase by after visiting each associated
+   *        list item, unless overridden by |value|.
+   * @param aForCounting Whether we are counting the elements or actually
+   *        restyling them. When true, this simply visits all children,
+   *        ignoring |<li value="..">| changes, effectively counting them
+   *        and storing the result in |aOrdinal|. This is useful for
+   *        |<ol reversed>|, where we need to count the number of
+   *        applicable child list elements before numbering. When false,
+   *        this will restyle all applicable descendants, and the next
+   *        ordinal value will be stored in |aOrdinal|, taking into account
+   *        any changes from |<li value="..">|.
+   * @param aDepth Current depth in frame tree from root list element.
+   */
   static bool RenumberListsInBlock(nsPresContext* aPresContext,
                                    nsBlockFrame* aBlockFrame,
                                    int32_t* aOrdinal,
                                    int32_t aDepth,
-                                   int32_t aIncrement);
+                                   int32_t aIncrement,
+                                   bool aForCounting);
 
+  /**
+   * Renumber the lists for a single frame.
+   * May recurse into RenumberListsInBlock.
+   * See RenumberListsInBlock for description of parameters.
+   */
   static bool RenumberListsFor(nsPresContext* aPresContext, nsIFrame* aKid,
                                int32_t* aOrdinal, int32_t aDepth,
-                               int32_t aIncrement);
+                               int32_t aIncrement,
+                               bool aForCounting);
 
   static bool FrameStartsCounterScope(nsIFrame* aFrame);
 
   void ReflowBullet(nsIFrame* aBulletFrame,
                     BlockReflowInput& aState,
                     ReflowOutput& aMetrics,
                     nscoord aLineTop);