Bug 601912. Implement <ol reversed>. r=dholbert
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 07 Sep 2012 22:30:24 -0400
changeset 108234 f083c0a666d3b82d776ed054568f48415635710f
parent 108231 34b091c8b0c844809eaaf1ecd7a2988ac9ed8b22
child 108235 4afed1cd79a61e26763bfd581d581f79eae7c590
push idunknown
push userunknown
push dateunknown
reviewersdholbert
bugs601912
milestone18.0a1
Bug 601912. Implement <ol reversed>. r=dholbert
content/html/content/src/nsHTMLOListElement.cpp
content/html/content/test/test_ol_attributes_reflection.html
dom/interfaces/html/nsIDOMHTMLOListElement.idl
layout/generic/nsBlockFrame.cpp
layout/generic/nsBlockFrame.h
layout/generic/nsBulletFrame.cpp
layout/generic/nsBulletFrame.h
layout/reftests/list-item/ol-reversed-1-ref.html
layout/reftests/list-item/ol-reversed-1a.html
layout/reftests/list-item/ol-reversed-1b.html
layout/reftests/list-item/ol-reversed-1c.html
layout/reftests/list-item/ol-reversed-2-ref.html
layout/reftests/list-item/ol-reversed-2.html
layout/reftests/list-item/ol-reversed-3-ref.html
layout/reftests/list-item/ol-reversed-3.html
layout/reftests/list-item/reftest.list
--- a/content/html/content/src/nsHTMLOListElement.cpp
+++ b/content/html/content/src/nsHTMLOListElement.cpp
@@ -120,16 +120,17 @@ NS_HTML_CONTENT_INTERFACE_MAP_END
 
 
 NS_IMPL_ELEMENT_CLONE(nsHTMLSharedListElement)
 
 
 NS_IMPL_BOOL_ATTR(nsHTMLSharedListElement, Compact, compact)
 NS_IMPL_INT_ATTR_DEFAULT_VALUE(nsHTMLSharedListElement, Start, start, 1)
 NS_IMPL_STRING_ATTR(nsHTMLSharedListElement, Type, type)
+NS_IMPL_BOOL_ATTR(nsHTMLSharedListElement, Reversed, reversed)
 
 // Shared with nsHTMLSharedElement.cpp
 nsAttrValue::EnumTable kListTypeTable[] = {
   { "none", NS_STYLE_LIST_STYLE_NONE },
   { "disc", NS_STYLE_LIST_STYLE_DISC },
   { "circle", NS_STYLE_LIST_STYLE_CIRCLE },
   { "round", NS_STYLE_LIST_STYLE_CIRCLE },
   { "square", NS_STYLE_LIST_STYLE_SQUARE },
--- a/content/html/content/test/test_ol_attributes_reflection.html
+++ b/content/html/content/test/test_ol_attributes_reflection.html
@@ -10,18 +10,21 @@
 <p id="display"></p>
 <div id="content" style="display: none">
 </div>
 <pre id="test">
 <script type="application/javascript">
 
 /** Test for HTMLOLElement attributes reflection **/
 
-// TODO: .reversed (boolean), bug 601912
-todo("reversed" in document.createElement("ol"), "reversed is not yet implemented");
+// .reversed (boolean)
+reflectBoolean({
+  element: document.createElement("ol"),
+  attribute: "reversed",
+})
 
 // .start
 reflectInt({
   element: document.createElement("ol"),
   attribute: "start",
   nonNegative: false,
   defaultValue: 1,
 });
--- a/dom/interfaces/html/nsIDOMHTMLOListElement.idl
+++ b/dom/interfaces/html/nsIDOMHTMLOListElement.idl
@@ -11,15 +11,16 @@
  *
  * This interface is trying to follow the DOM Level 2 HTML specification:
  * http://www.w3.org/TR/DOM-Level-2-HTML/
  *
  * with changes from the work-in-progress WHATWG HTML specification:
  * http://www.whatwg.org/specs/web-apps/current-work/
  */
 
-[scriptable, uuid(64155DCA-83CA-4FFE-8B64-A7F82F29586F)]
+[scriptable, uuid(8A2BA7C5-B3F2-4A57-86F9-0E87C291A591)]
 interface nsIDOMHTMLOListElement : nsIDOMHTMLElement
 {
            attribute boolean          compact;
+           attribute boolean          reversed;
            attribute long             start;
            attribute DOMString        type;
 };
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -2782,17 +2782,18 @@ nsBlockFrame::AttributeChanged(int32_t  
                                int32_t         aModType)
 {
   nsresult rv = nsBlockFrameSuper::AttributeChanged(aNameSpaceID,
                                                     aAttribute, aModType);
 
   if (NS_FAILED(rv)) {
     return rv;
   }
-  if (nsGkAtoms::start == aAttribute) {
+  if (nsGkAtoms::start == aAttribute ||
+      (nsGkAtoms::reversed == aAttribute && mContent->IsHTML(nsGkAtoms::ol))) {
     nsPresContext* presContext = PresContext();
 
     // XXX Not sure if this is necessary anymore
     if (RenumberLists(presContext)) {
       presContext->PresShell()->
         FrameNeedsReflow(this, nsIPresShell::eStyleChange,
                          NS_FRAME_HAS_DIRTY_CHILDREN);
     }
@@ -6711,71 +6712,94 @@ bool
 nsBlockFrame::RenumberLists(nsPresContext* aPresContext)
 {
   if (!FrameStartsCounterScope(this)) {
     // If this frame doesn't start a counter scope then we don't need
     // to renumber child list items.
     return false;
   }
 
+  MOZ_ASSERT(mContent->IsHTML(),
+             "FrameStartsCounterScope should only return true for HTML elements");
+
   // Setup initial list ordinal value
   // XXX Map html's start property to counter-reset style
   int32_t ordinal = 1;
+  int32_t increment;
+  if (mContent->Tag() == nsGkAtoms::ol &&
+      mContent->HasAttr(kNameSpaceID_None, nsGkAtoms::reversed)) {
+    increment = -1;
+  } else {
+    increment = 1;
+  }
 
   nsGenericHTMLElement *hc = nsGenericHTMLElement::FromContent(mContent);
-
-  if (hc) {
-    const nsAttrValue* attr = hc->GetParsedAttr(nsGkAtoms::start);
-    if (attr && attr->Type() == nsAttrValue::eInteger) {
-      ordinal = attr->GetIntegerValue();
+  // Must be non-null, since FrameStartsCounterScope only returns true
+  // for HTML elements.
+  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->IsHTML(nsGkAtoms::li)) {
+        ordinal -= increment;
+      }
     }
   }
 
   // Get to first-in-flow
   nsBlockFrame* block = (nsBlockFrame*) GetFirstInFlow();
-  return RenumberListsInBlock(aPresContext, block, &ordinal, 0);
+  return RenumberListsInBlock(aPresContext, block, &ordinal, 0, increment);
 }
 
 bool
 nsBlockFrame::RenumberListsInBlock(nsPresContext* aPresContext,
                                    nsBlockFrame* aBlockFrame,
                                    int32_t* aOrdinal,
-                                   int32_t aDepth)
+                                   int32_t aDepth,
+                                   int32_t aIncrement)
 {
   // 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);
+      bool kidRenumberedABullet = RenumberListsFor(aPresContext, kid, aOrdinal,
+                                                   aDepth, aIncrement);
       if (kidRenumberedABullet) {
         line->MarkDirty();
         renumberedABullet = true;
       }
       kid = kid->GetNextSibling();
     }
   } while (bifLineIter.Next());
 
   return renumberedABullet;
 }
 
 bool
 nsBlockFrame::RenumberListsFor(nsPresContext* aPresContext,
                                nsIFrame* aKid,
                                int32_t* aOrdinal,
-                               int32_t aDepth)
+                               int32_t aDepth,
+                               int32_t aIncrement)
 {
   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
@@ -6797,46 +6821,49 @@ nsBlockFrame::RenumberListsFor(nsPresCon
   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);
+        *aOrdinal = bullet->SetListItemOrdinal(*aOrdinal, &changed, aIncrement);
         if (changed) {
           kidRenumberedABullet = true;
 
           // The ordinal changed - mark the bullet frame dirty.
           listItem->ChildIsDirty(bullet);
         }
       }
 
       // 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);
+      bool meToo = RenumberListsInBlock(aPresContext, listItem, aOrdinal,
+                                        aDepth + 1, aIncrement);
       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
       // counter scope. Any list-items in there will be handled by
       // it.
     }
     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);
+        kidRenumberedABullet = RenumberListsInBlock(aPresContext, kidBlock,
+                                                    aOrdinal, aDepth + 1,
+                                                    aIncrement);
       }
     }
   }
   return kidRenumberedABullet;
 }
 
 void
 nsBlockFrame::ReflowBullet(nsIFrame* aBulletFrame,
--- a/layout/generic/nsBlockFrame.h
+++ b/layout/generic/nsBlockFrame.h
@@ -686,21 +686,24 @@ protected:
 
   // 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);
 
   static bool RenumberListsInBlock(nsPresContext* aPresContext,
-                                     nsBlockFrame* aBlockFrame,
-                                     int32_t* aOrdinal,
-                                     int32_t aDepth);
+                                   nsBlockFrame* aBlockFrame,
+                                   int32_t* aOrdinal,
+                                   int32_t aDepth,
+                                   int32_t aIncrement);
 
-  static bool RenumberListsFor(nsPresContext* aPresContext, nsIFrame* aKid, int32_t* aOrdinal, int32_t aDepth);
+  static bool RenumberListsFor(nsPresContext* aPresContext, nsIFrame* aKid,
+                               int32_t* aOrdinal, int32_t aDepth,
+                               int32_t aIncrement);
 
   static bool FrameStartsCounterScope(nsIFrame* aFrame);
 
   void ReflowBullet(nsIFrame* aBulletFrame,
                     nsBlockReflowState& aState,
                     nsHTMLReflowMetrics& aMetrics,
                     nscoord aLineTop);
 
--- a/layout/generic/nsBulletFrame.cpp
+++ b/layout/generic/nsBulletFrame.cpp
@@ -347,18 +347,22 @@ nsBulletFrame::PaintBullet(nsRenderingCo
     aRenderingContext.DrawString(text, mPadding.left + aPt.x,
                                  mPadding.top + aPt.y + ascent);
     break;
   }
 }
 
 int32_t
 nsBulletFrame::SetListItemOrdinal(int32_t aNextOrdinal,
-                                  bool* aChanged)
+                                  bool* aChanged,
+                                  int32_t aIncrement)
 {
+  MOZ_ASSERT(aIncrement == 1 || aIncrement == -1,
+             "We shouldn't have weird increments here");
+
   // Assume that the ordinal comes from the caller
   int32_t oldOrdinal = mOrdinal;
   mOrdinal = aNextOrdinal;
 
   // Try to get value directly from the list-item, if it specifies a
   // value attribute. Note: we do this with our parent's content
   // because our parent is the list-item.
   nsIContent* parentContent = mParent->GetContent();
@@ -371,17 +375,17 @@ nsBulletFrame::SetListItemOrdinal(int32_
         // Use ordinal specified by the value attribute
         mOrdinal = attr->GetIntegerValue();
       }
     }
   }
 
   *aChanged = oldOrdinal != mOrdinal;
 
-  return mOrdinal + 1;
+  return mOrdinal + aIncrement;
 }
 
 
 // XXX change roman/alpha to use unsigned math so that maxint and
 // maxnegint will work
 
 /**
  * For all functions below, a return value of true means that we
--- a/layout/generic/nsBulletFrame.h
+++ b/layout/generic/nsBulletFrame.h
@@ -75,17 +75,18 @@ public:
   NS_IMETHOD Reflow(nsPresContext* aPresContext,
                     nsHTMLReflowMetrics& aMetrics,
                     const nsHTMLReflowState& aReflowState,
                     nsReflowStatus& aStatus);
   virtual nscoord GetMinWidth(nsRenderingContext *aRenderingContext);
   virtual nscoord GetPrefWidth(nsRenderingContext *aRenderingContext);
 
   // nsBulletFrame
-  int32_t SetListItemOrdinal(int32_t aNextOrdinal, bool* aChanged);
+  int32_t SetListItemOrdinal(int32_t aNextOrdinal, bool* aChanged,
+                             int32_t aIncrement);
 
 
   NS_IMETHOD OnStartContainer(imgIRequest *aRequest, imgIContainer *aImage);
   NS_IMETHOD OnDataAvailable(imgIRequest *aRequest,
                              bool aCurrentFrame,
                              const nsIntRect *aRect);
   NS_IMETHOD OnStopDecode(imgIRequest *aRequest,
                           nsresult aStatus,
new file mode 100644
--- /dev/null
+++ b/layout/reftests/list-item/ol-reversed-1-ref.html
@@ -0,0 +1,6 @@
+<!DOCTYPE html>
+<ol>
+  <li value="3">Three</li>
+  <li value="2">Two</li>
+  <li value="1">One</li>
+</ol>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/list-item/ol-reversed-1a.html
@@ -0,0 +1,6 @@
+<!DOCTYPE html>
+<ol reversed>
+  <li>Three</li>
+  <li>Two</li>
+  <li>One</li>
+</ol>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/list-item/ol-reversed-1b.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<ol id="x">
+  <li>Three</li>
+  <li>Two</li>
+  <li>One</li>
+</ol>
+<script>
+  var l = document.getElementById("x");
+  var w = l.offsetWidth;
+  l.setAttribute("reversed", "");
+</script>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/list-item/ol-reversed-1c.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<ol id="x" reversed>
+  <li>Three</li>
+  <li>Two</li>
+</ol>
+<script>
+  var l = document.getElementById("x");
+  var w = l.offsetWidth;
+  var li = document.createElement("li");
+  li.textContent = "One"
+  l.appendChild(li);
+</script>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/list-item/ol-reversed-2-ref.html
@@ -0,0 +1,6 @@
+<!DOCTYPE html>
+<ol>
+  <li value="5">Five</li>
+  <li value="4">Four</li>
+  <li value="3">Three</li>
+</ol>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/list-item/ol-reversed-2.html
@@ -0,0 +1,6 @@
+<!DOCTYPE html>
+<ol reversed start="5">
+  <li>Five</li>
+  <li>Four</li>
+  <li>Three</li>
+</ol>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/list-item/ol-reversed-3-ref.html
@@ -0,0 +1,6 @@
+<!DOCTYPE html>
+<ol>
+  <li value="1">One</li>
+  <li value="0">Zero</li>
+  <li value="-1">Neg-One</li>
+</ol>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/list-item/ol-reversed-3.html
@@ -0,0 +1,6 @@
+<!DOCTYPE html>
+<ol reversed start="1">
+  <li>One</li>
+  <li>Zero</li>
+  <li>Neg-One</li>
+</ol>
--- a/layout/reftests/list-item/reftest.list
+++ b/layout/reftests/list-item/reftest.list
@@ -1,2 +1,7 @@
 == numbering-1.html numbering-1-ref.html
 == numbering-2.html numbering-2-ref.html
+== ol-reversed-1a.html ol-reversed-1-ref.html
+asserts(1) == ol-reversed-1b.html ol-reversed-1-ref.html # bug 478135
+== ol-reversed-1c.html ol-reversed-1-ref.html
+== ol-reversed-2.html ol-reversed-2-ref.html
+== ol-reversed-3.html ol-reversed-3-ref.html