Bug 601912. Implement <ol reversed>. r=dholbert

2024-10-09 03:15:11 +00:00 · 2012-09-07 22:30:24 -04:00 · 2012-09-07 22:30:24 -04:00 · 942cc93041
commit 942cc93041
parent d8e0dcfd02
16 changed files with 127 additions and 23 deletions
--- a/content/html/content/src/nsHTMLOListElement.cpp
+++ b/content/html/content/src/nsHTMLOListElement.cpp
@ -125,6 +125,7 @@ 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[] = {
--- a/content/html/content/test/test_ol_attributes_reflection.html
+++ b/content/html/content/test/test_ol_attributes_reflection.html
@ -15,8 +15,11 @@

 /** 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({
--- a/dom/interfaces/html/nsIDOMHTMLOListElement.idl
+++ b/dom/interfaces/html/nsIDOMHTMLOListElement.idl
@ -16,10 +16,11 @@
 * 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
@ -2787,7 +2787,8 @@ nsBlockFrame::AttributeChanged(int32_t         aNameSpaceID,
  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
@ -6716,29 +6717,50 @@ nsBlockFrame::RenumberLists(nsPresContext* aPresContext)
    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;
@ -6754,7 +6776,8 @@ nsBlockFrame::RenumberListsInBlock(nsPresContext* aPresContext,
    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;
@ -6770,7 +6793,8 @@ 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!");

@ -6802,7 +6826,7 @@ nsBlockFrame::RenumberListsFor(nsPresContext* aPresContext,
      nsBulletFrame* bullet = listItem->GetBullet();
      if (bullet) {
        bool changed;
-        *aOrdinal = bullet->SetListItemOrdinal(*aOrdinal, &changed);
+        *aOrdinal = bullet->SetListItemOrdinal(*aOrdinal, &changed, aIncrement);
        if (changed) {
          kidRenumberedABullet = true;

@ -6814,7 +6838,8 @@ nsBlockFrame::RenumberListsFor(nsPresContext* aPresContext,
      // 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;
      }
@ -6831,7 +6856,9 @@ nsBlockFrame::RenumberListsFor(nsPresContext* aPresContext,
      // 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);
      }
    }
  }
--- a/layout/generic/nsBlockFrame.h
+++ b/layout/generic/nsBlockFrame.h
@ -691,11 +691,14 @@ protected:
  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);

--- a/layout/generic/nsBulletFrame.cpp
+++ b/layout/generic/nsBulletFrame.cpp
@ -352,8 +352,12 @@ nsBulletFrame::PaintBullet(nsRenderingContext& aRenderingContext, nsPoint aPt,

 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;
@ -376,7 +380,7 @@ nsBulletFrame::SetListItemOrdinal(int32_t aNextOrdinal,

  *aChanged = oldOrdinal != mOrdinal;

-  return mOrdinal + 1;
+  return mOrdinal + aIncrement;
 }


--- a/layout/generic/nsBulletFrame.h
+++ b/layout/generic/nsBulletFrame.h
@ -80,7 +80,8 @@ public:
  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);
--- a/layout/reftests/list-item/ol-reversed-1-ref.html
+++ 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>
--- a/layout/reftests/list-item/ol-reversed-1a.html
+++ 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>
--- a/layout/reftests/list-item/ol-reversed-1b.html
+++ 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>
--- a/layout/reftests/list-item/ol-reversed-1c.html
+++ 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>
--- a/layout/reftests/list-item/ol-reversed-2-ref.html
+++ 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>
--- a/layout/reftests/list-item/ol-reversed-2.html
+++ 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>
--- a/layout/reftests/list-item/ol-reversed-3-ref.html
+++ 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>
--- a/layout/reftests/list-item/ol-reversed-3.html
+++ 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