Bug 417255. Rework getClientRects/getBoundingClientRect/offset* code to use a generic rectangle iterator API which drills down through anonymous blocks, fixing IE compat. r+sr=dbaron

content/base/src/nsGenericElement.cpp

@@ -810,19 +810,16 @@ nsNSElementTearoff::GetElementsByClassName(const nsAString& aClasses,
   return nsDocument::GetElementsByClassNameHelper(mContent, aClasses, aReturn);
 }
 
-static nsPoint
-GetOffsetFromInitialContainingBlock(nsIFrame* aFrame)
+static nsIFrame*
+GetContainingBlockForClientRect(nsIFrame* aFrame)
 {
-  nsIFrame* refFrame = aFrame->GetParent();
-  if (!refFrame)
-    return nsPoint(0, 0);
-
   // get the nearest enclosing SVG foreign object frame or the root frame
-  while (refFrame->GetParent() &&
-         !refFrame->IsFrameOfType(nsIFrame::eSVGForeignObject))
-    refFrame = refFrame->GetParent();
+  while (aFrame->GetParent() &&
+         !aFrame->IsFrameOfType(nsIFrame::eSVGForeignObject)) {
+    aFrame = aFrame->GetParent();
+  }
 
-  return aFrame->GetOffsetTo(refFrame);
+  return aFrame;
 }
 
 static double
@@ -847,25 +844,6 @@ SetTextRectangle(const nsRect& aLayoutRect, nsPresContext* aPresContext,
                  RoundFloat(aLayoutRect.YMost()*t2pScaled)*scaleInv - y);
 }
 
-static PRBool
-TryGetSVGBoundingRect(nsIFrame* aFrame, nsRect* aRect)
-{
-#ifdef MOZ_SVG
-  nsRect r;
-  nsIFrame* outer = nsSVGUtils::GetOuterSVGFrameAndCoveredRegion(aFrame, &r);
-  if (!outer)
-    return PR_FALSE;
-
-  // r is in pixels relative to 'outer', get it into twips
-  // relative to ICB origin
-  r.ScaleRoundOut(1.0/aFrame->PresContext()->AppUnitsPerDevPixel());
-  *aRect = r + GetOffsetFromInitialContainingBlock(outer);
-  return PR_TRUE;
-#else
-  return PR_FALSE;
-#endif
-}
-
 NS_IMETHODIMP
 nsNSElementTearoff::GetBoundingClientRect(nsIDOMTextRectangle** aResult)
 {
@@ -881,75 +859,44 @@ nsNSElementTearoff::GetBoundingClientRect(nsIDOMTextRectangle** aResult)
     // display:none, perhaps? Return the empty rect
     return NS_OK;
   }
+
   nsPresContext* presContext = frame->PresContext();
-  
-  nsRect r;
-  if (TryGetSVGBoundingRect(frame, &r)) {
-    // Currently SVG frames don't have continuations but I don't want things to
-    // break if that changes.
-    while ((frame = nsLayoutUtils::GetNextContinuationOrSpecialSibling(frame)) != nsnull) {
-      nsRect nextRect;
-#ifdef DEBUG
-      PRBool isSVG =
-#endif
-        TryGetSVGBoundingRect(frame, &nextRect);
-      NS_ASSERTION(isSVG, "SVG frames must have SVG continuations");
-      r.UnionRect(r, nextRect);
-    }
-  } else {
-    // The weird frame layout of tables requires this
-    if (frame->GetType() == nsGkAtoms::tableOuterFrame) {
-      nsIFrame* innerTable = frame->GetFirstChild(nsnull);
-      if (innerTable) {
-        r = nsLayoutUtils::GetAllInFlowBoundingRect(innerTable) + innerTable->GetPosition();
-      }
-      nsIFrame* caption = frame->GetFirstChild(nsGkAtoms::captionList);
-      if (caption) {
-        r.UnionRect(r, nsLayoutUtils::GetAllInFlowBoundingRect(caption) + caption->GetPosition());
-      }
-    } else {
-      r = nsLayoutUtils::GetAllInFlowBoundingRect(frame);
-    }
-    r += GetOffsetFromInitialContainingBlock(frame);
-  }
+  nsRect r = nsLayoutUtils::GetAllInFlowRectsUnion(frame,
+          GetContainingBlockForClientRect(frame));
   SetTextRectangle(r, presContext, rect);
   return NS_OK;
 }
 
-static nsresult
-AddRectanglesForFrames(nsTextRectangleList* aRectList, nsIFrame* aFrame)
-{
-  if (!aFrame)
-    return NS_OK;
+struct RectListBuilder : public nsLayoutUtils::RectCallback {
+  nsPresContext*       mPresContext;
+  nsTextRectangleList* mRectList;
+  nsresult             mRV;
 
-  nsPresContext* presContext = aFrame->PresContext();
-  for (nsIFrame* f = aFrame; f;
-       f = nsLayoutUtils::GetNextContinuationOrSpecialSibling(f)) {
+  RectListBuilder(nsPresContext* aPresContext, nsTextRectangleList* aList) 
+    : mPresContext(aPresContext), mRectList(aList),
+      mRV(NS_OK) {}
+
+  virtual void AddRect(const nsRect& aRect) {
     nsRefPtr<nsTextRectangle> rect = new nsTextRectangle();
-    if (!rect)
-      return NS_ERROR_OUT_OF_MEMORY;
-    
-    nsRect r;
-    if (!TryGetSVGBoundingRect(f, &r)) {
-      r = nsRect(GetOffsetFromInitialContainingBlock(f), f->GetSize());
+    if (!rect) {
+      mRV = NS_ERROR_OUT_OF_MEMORY;
+      return;
     }
-    SetTextRectangle(r, presContext, rect);
-    aRectList->Append(rect);
+    
+    SetTextRectangle(aRect, mPresContext, rect);
+    mRectList->Append(rect);
   }
-
-  return NS_OK;
-}
+};
 
 NS_IMETHODIMP
 nsNSElementTearoff::GetClientRects(nsIDOMTextRectangleList** aResult)
 {
   *aResult = nsnull;
 
-  // Weak ref, since we addref it below
   nsRefPtr<nsTextRectangleList> rectList = new nsTextRectangleList();
   if (!rectList)
     return NS_ERROR_OUT_OF_MEMORY;
-  
+
   nsIFrame* frame = mContent->GetPrimaryFrame(Flush_Layout);
   if (!frame) {
     // display:none, perhaps? Return an empty list
@@ -957,21 +904,11 @@ nsNSElementTearoff::GetClientRects(nsIDOMTextRectangleList** aResult)
     return NS_OK;
   }
 
-  if (frame->GetType() == nsGkAtoms::tableOuterFrame) {
-    // The weird frame layout of tables requires this
-    nsIFrame* innerTable = frame->GetFirstChild(nsnull);
-    nsresult rv = AddRectanglesForFrames(rectList, innerTable);
-    if (NS_FAILED(rv))
-      return rv;
-    nsIFrame* caption = frame->GetFirstChild(nsGkAtoms::captionList);
-    rv = AddRectanglesForFrames(rectList, caption);
-    if (NS_FAILED(rv))
-      return rv;
-  } else {
-    nsresult rv = AddRectanglesForFrames(rectList, frame);
-    if (NS_FAILED(rv))
-      return rv;
-  }
+  RectListBuilder builder(frame->PresContext(), rectList);
+  nsLayoutUtils::GetAllInFlowRects(frame,
+          GetContainingBlockForClientRect(frame), &builder);
+  if (NS_FAILED(builder.mRV))
+    return builder.mRV;
   *aResult = rectList.forget().get();
   return NS_OK;
 }

content/base/test/Makefile.in

@@ -170,6 +170,7 @@ _TEST_FILES = 	test_bug5141.html \
 		test_bug414190.html \
 		test_bug414796.html \
 		test_bug416383.html \
+		test_bug417255.html \
 		test_bug417384.html \
 		test_bug418214.html \
 		$(NULL)

content/base/test/test_bug417255.html

@@ -0,0 +1,61 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=417255
+-->
+<head>
+  <title>Test for Bug 417255</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+  <style>
+  .spacer { display:inline-block; height:10px; }
+  </style>
+</head>
+<body>
+
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=417255">Mozilla Bug 417255</a>
+<p id="display" style="width:800px"></p>
+
+<p><span id="s1" style="border:2px dotted red;"><span class="spacer" style="width:100px"></span>
+<div style="width:500px; height:100px; background:yellow;"></div>
+<span class="spacer" style="width:200px"></span></span>
+
+<p><span id="s2" style="border:2px dotted red;"><span class="spacer" style="width:100px"></span>
+<div style="width:150px; height:100px; background:yellow;"></div>
+<span class="spacer" style="width:200px"></span></span>
+
+<!-- test nested spans around the IB split -->
+<p><span id="s3" style="border:2px dotted red;"><span><span class="spacer" style="width:100px"></span>
+<div style="width:500px; height:100px; background:yellow;"></div>
+<span class="spacer" style="width:200px"></span></span></span>
+
+<div id="content" style="display: none">
+
+</div>
+
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+function getWidth(box) {
+  return box.right - box.left;
+}
+
+function doTest(id, boundsWidth, w1, w2, w3) {
+  var s = document.getElementById(id);
+  is(s.offsetWidth, boundsWidth, "bad offsetWidth");
+  is(getWidth(s.getBoundingClientRect()), boundsWidth, "bad getBoundingClientRect width");
+  is(getWidth(s.getClientRects()[0]), w1, "bad getClientRects width"); 
+  is(getWidth(s.getClientRects()[1]), w2, "bad getClientRects width"); 
+  is(getWidth(s.getClientRects()[2]), w3, "bad getClientRects width"); 
+}
+
+doTest("s1", 500, 104, 500, 204);
+doTest("s2", 204, 104, 150, 204);
+doTest("s3", 500, 104, 500, 204);
+
+</script>
+</pre>
+</body>
+
+</html>

content/html/content/src/nsGenericHTMLElement.cpp

@@ -512,8 +512,6 @@ nsGenericHTMLElement::GetOffsetRect(nsRect& aRect, nsIContent** aOffsetParent)
     parent = parent->GetParent();
   }
 
-  // Get the union of all rectangles in this and continuation frames.
-  nsRect rcFrame = nsLayoutUtils::GetAllInFlowBoundingRect(frame);
   nsIContent* docElement = GetCurrentDoc()->GetRootContent();
   nsIContent* content = frame->GetContent();
 
@@ -594,6 +592,12 @@ nsGenericHTMLElement::GetOffsetRect(nsRect& aRect, nsIContent** aOffsetParent)
   // Convert to pixels.
   aRect.x = nsPresContext::AppUnitsToIntCSSPixels(origin.x);
   aRect.y = nsPresContext::AppUnitsToIntCSSPixels(origin.y);
+
+  // Get the union of all rectangles in this and continuation frames.
+  // It doesn't really matter what we use as aRelativeTo here, since
+  // we only care about the size. Using 'parent' might make things
+  // a bit faster by speeding up the internal GetOffsetTo operations.
+  nsRect rcFrame = nsLayoutUtils::GetAllInFlowRectsUnion(frame, parent);
   aRect.width = nsPresContext::AppUnitsToIntCSSPixels(rcFrame.width);
   aRect.height = nsPresContext::AppUnitsToIntCSSPixels(rcFrame.height);
 }

layout/base/nsLayoutUtils.cpp

@@ -47,6 +47,7 @@
 #include "nsGkAtoms.h"
 #include "nsIAtom.h"
 #include "nsCSSPseudoElements.h"
+#include "nsCSSAnonBoxes.h"
 #include "nsIView.h"
 #include "nsIScrollableView.h"
 #include "nsPlaceholderFrame.h"
@@ -70,9 +71,11 @@
 #include "nsCSSRendering.h"
 #include "nsContentUtils.h"
 
+#ifdef MOZ_SVG
+#include "nsSVGUtils.h"
+#endif
 #ifdef MOZ_SVG_FOREIGNOBJECT
 #include "nsSVGForeignObjectFrame.h"
-#include "nsSVGUtils.h"
 #include "nsSVGOuterSVGFrame.h"
 #endif
 
@@ -1080,27 +1083,73 @@ nsLayoutUtils::BinarySearchForPosition(nsIRenderingContext* aRendContext,
   return PR_FALSE;
 }
 
-nsRect
-nsLayoutUtils::GetAllInFlowBoundingRect(nsIFrame* aFrame)
+static void
+AddRectsForFrame(nsIFrame* aFrame, nsIFrame* aRelativeTo,
+                 nsLayoutUtils::RectCallback* aCallback)
 {
-  // Get the union of all rectangles in this and continuation frames
-  nsRect r = aFrame->GetRect();
-  nsIFrame* parent = aFrame->GetParent();
-  if (!parent)
-    return r;
+  nsIAtom* pseudoType = aFrame->GetStyleContext()->GetPseudoType();
 
-  for (nsIFrame* f = nsLayoutUtils::GetNextContinuationOrSpecialSibling(aFrame);
-       f; f = nsLayoutUtils::GetNextContinuationOrSpecialSibling(f)) {
-    r.UnionRect(r, nsRect(f->GetOffsetTo(parent), f->GetSize()));
+  if (pseudoType == nsCSSAnonBoxes::tableOuter) {
+    AddRectsForFrame(aFrame->GetFirstChild(nsnull), aRelativeTo,
+                     aCallback);
+    nsIFrame* kid = aFrame->GetFirstChild(nsGkAtoms::captionList);
+    if (kid) {
+      AddRectsForFrame(kid, aRelativeTo, aCallback);
+    }
+  } else if (pseudoType == nsCSSAnonBoxes::mozAnonymousBlock ||
+             pseudoType == nsCSSAnonBoxes::mozAnonymousPositionedBlock ||
+             pseudoType == nsCSSAnonBoxes::mozMathMLAnonymousBlock ||
+             pseudoType == nsCSSAnonBoxes::mozXULAnonymousBlock) {
+    for (nsIFrame* kid = aFrame->GetFirstChild(nsnull); kid; kid = kid->GetNextSibling()) {
+      AddRectsForFrame(kid, aRelativeTo, aCallback);
+    }
+  } else {
+#ifdef MOZ_SVG
+    nsRect r;
+    nsIFrame* outer = nsSVGUtils::GetOuterSVGFrameAndCoveredRegion(aFrame, &r);
+    if (outer) {
+      // r is in pixels relative to 'outer', get it into appunits
+      // relative to aRelativeTo
+      r.ScaleRoundOut(1.0/aFrame->PresContext()->AppUnitsPerDevPixel());
+      aCallback->AddRect(r + outer->GetOffsetTo(aRelativeTo));
+    } else
+#endif
+      aCallback->AddRect(nsRect(aFrame->GetOffsetTo(aRelativeTo), aFrame->GetSize()));
   }
+}
 
-  if (r.IsEmpty()) {
-    // It could happen that all the rects are empty (eg zero-width or
-    // zero-height).  In that case, use the first rect for the frame.
-    r = aFrame->GetRect();
+void
+nsLayoutUtils::GetAllInFlowRects(nsIFrame* aFrame, nsIFrame* aRelativeTo,
+                                 RectCallback* aCallback)
+{
+  while (aFrame) {
+    AddRectsForFrame(aFrame, aRelativeTo, aCallback);
+    aFrame = nsLayoutUtils::GetNextContinuationOrSpecialSibling(aFrame);
   }
+}
 
-  return r - aFrame->GetPosition();
+struct RectAccumulator : public nsLayoutUtils::RectCallback {
+  nsRect       mResultRect;
+  nsRect       mFirstRect;
+  PRPackedBool mSeenFirstRect;
+
+  RectAccumulator() : mSeenFirstRect(PR_FALSE) {}
+
+  virtual void AddRect(const nsRect& aRect) {
+    mResultRect.UnionRect(mResultRect, aRect);
+    if (!mSeenFirstRect) {
+      mSeenFirstRect = PR_TRUE;
+      mFirstRect = aRect;
+    }
+  }
+};
+
+nsRect
+nsLayoutUtils::GetAllInFlowRectsUnion(nsIFrame* aFrame, nsIFrame* aRelativeTo) {
+  RectAccumulator accumulator;
+  GetAllInFlowRects(aFrame, aRelativeTo, &accumulator);
+  return accumulator.mResultRect.IsEmpty() ? accumulator.mFirstRect
+          : accumulator.mResultRect;
 }
 
 nsresult

layout/base/nsLayoutUtils.h

@@ -471,13 +471,27 @@ public:
                           PRInt32&   aIndex,
                           PRInt32&   aTextWidth);
 
+  class RectCallback {
+  public:
+    virtual void AddRect(const nsRect& aRect) = 0;
+  };
   /**
-   * Get the union of all rects in aFrame and its continuations, relative
-   * to aFrame's origin. Scrolling is taken into account, but this shouldn't
-   * matter because it should be impossible to have some continuations scrolled
-   * differently from others.
+   * Collect all CSS border-boxes associated with aFrame and its
+   * continuations, "drilling down" through outer table frames and
+   * some anonymous blocks since they're not real CSS boxes.
+   * The boxes are positioned relative to aRelativeTo (taking scrolling
+   * into account) and passed to the callback in frame-tree order.
+   * If aFrame is null, no boxes are returned.
+   * For SVG frames, returns one rectangle, the bounding box.
    */
-  static nsRect GetAllInFlowBoundingRect(nsIFrame* aFrame);
+  static void GetAllInFlowRects(nsIFrame* aFrame, nsIFrame* aRelativeTo,
+                                RectCallback* aCallback);
+
+  /**
+   * Computes the union of all rects returned by GetAllInFlowRects. If
+   * the union is empty, returns the first rect.
+   */
+  static nsRect GetAllInFlowRectsUnion(nsIFrame* aFrame, nsIFrame* aRelativeTo);
 
   /**
    * Get the font metrics corresponding to the frame's style data.

layout/xul/base/src/nsBoxObject.cpp

@@ -173,9 +173,6 @@ nsBoxObject::GetOffsetRect(nsRect& aRect)
     // Get its origin
     nsPoint origin = frame->GetPositionIgnoringScrolling();
 
-    // Get the union of all rectangles in this and continuation frames
-    nsRect rcFrame = nsLayoutUtils::GetAllInFlowBoundingRect(frame);
-        
     // Find the frame parent whose content is the document element.
     nsIContent *docElement = mContent->GetCurrentDoc()->GetRootContent();
     nsIFrame* parent = frame->GetParent();
@@ -210,10 +207,16 @@ nsBoxObject::GetOffsetRect(nsRect& aRect)
 
     aRect.x = nsPresContext::AppUnitsToIntCSSPixels(origin.x);
     aRect.y = nsPresContext::AppUnitsToIntCSSPixels(origin.y);
+    
+    // Get the union of all rectangles in this and continuation frames.
+    // It doesn't really matter what we use as aRelativeTo here, since
+    // we only care about the size. Using 'parent' might make things
+    // a bit faster by speeding up the internal GetOffsetTo operations.
+    nsRect rcFrame = nsLayoutUtils::GetAllInFlowRectsUnion(frame, parent);
     aRect.width = nsPresContext::AppUnitsToIntCSSPixels(rcFrame.width);
     aRect.height = nsPresContext::AppUnitsToIntCSSPixels(rcFrame.height);
   }
- 
+
   return NS_OK;
 }