Bug 1290813 - Correctly number indirect descendants of <ol reversed>; r=xidorn
authorManish Goregaokar <manishearth@gmail.com>
Mon, 01 Aug 2016 15:54:25 +0530
changeset 307519 2248a75304770fcb2b0eba29c7eb73a5a19fea43
parent 307518 1afdac38f42a552585cd9580e6619b47eeeddb0b
child 307520 738847eb38313539c6efc7678a53d84ee9fb0266
push id30514
push usercbook@mozilla.com
push dateTue, 02 Aug 2016 15:04:11 +0000
treeherdermozilla-central@ea6e87bbd03e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersxidorn
bugs1290813
milestone50.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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);