Bug 409111, WRONG_DOCUMENT_ERR on unattached nodes through getBoxObjectFor, r+sr=sicking, a=mtschrep

content/base/src/nsDocument.cpp

@@ -785,6 +785,15 @@ nsDocument::nsDocument(const char* aContentType)
   SetDOMStringToNull(mLastStyleSheetSet);
 }
 
+PR_STATIC_CALLBACK(PLDHashOperator)
+ClearAllBoxObjects(const void* aKey, nsPIBoxObject* aBoxObject, void* aUserArg)
+{
+  if (aBoxObject) {
+    aBoxObject->Clear();
+  }
+  return PL_DHASH_NEXT;
+}
+
 nsDocument::~nsDocument()
 {
 #ifdef PR_LOGGING
@@ -876,7 +885,12 @@ nsDocument::~nsDocument()
   }
 
   delete mHeaderData;
-  delete mBoxObjectTable;
+
+  if (mBoxObjectTable) {
+    mBoxObjectTable->EnumerateRead(ClearAllBoxObjects, nsnull);
+    delete mBoxObjectTable;
+  }
+
   delete mContentWrapperHash;
 }
 
@@ -919,12 +933,11 @@ RadioGroupsTraverser(const nsAString& aKey, nsAutoPtr<nsRadioGroupStruct>& aData
 }
 
 PR_STATIC_CALLBACK(PLDHashOperator)
-BoxObjectTraverser(nsISupports* key, nsPIBoxObject* boxObject, void* userArg)
+BoxObjectTraverser(const void* key, nsPIBoxObject* boxObject, void* userArg)
 {
   nsCycleCollectionTraversalCallback *cb = 
     static_cast<nsCycleCollectionTraversalCallback*>(userArg);
  
-  cb->NoteXPCOMChild(key);
   cb->NoteXPCOMChild(boxObject);
 
   return PL_DHASH_NEXT;
@@ -3662,14 +3675,25 @@ nsDocument::GetBoxObjectFor(nsIDOMElement* aElement, nsIBoxObject** aResult)
   nsCOMPtr<nsIContent> content(do_QueryInterface(aElement));
   NS_ENSURE_TRUE(content, NS_ERROR_UNEXPECTED);
 
-  nsIDocument* doc = content->HasFlag(NODE_FORCE_XBL_BINDINGS) ?
-    content->GetOwnerDoc() : content->GetCurrentDoc();
+  nsIDocument* doc = content->GetOwnerDoc();
   NS_ENSURE_TRUE(doc == this, NS_ERROR_DOM_WRONG_DOCUMENT_ERR);
-  
+
+  if (!mHasWarnedAboutBoxObjects && !content->IsNodeOfType(eXUL)) {
+    mHasWarnedAboutBoxObjects = PR_TRUE;
+    nsContentUtils::ReportToConsole(nsContentUtils::eDOM_PROPERTIES,
+                                    "UseOfGetBoxObjectForWarning",
+                                    nsnull, 0,
+                                    static_cast<nsIDocument*>(this)->
+                                      GetDocumentURI(),
+                                    EmptyString(), 0, 0,
+                                    nsIScriptError::warningFlag,
+                                    "BoxObjects");
+  }
+
   *aResult = nsnull;
 
   if (!mBoxObjectTable) {
-    mBoxObjectTable = new nsInterfaceHashtable<nsISupportsHashKey, nsPIBoxObject>;
+    mBoxObjectTable = new nsInterfaceHashtable<nsVoidPtrHashKey, nsPIBoxObject>;
     if (mBoxObjectTable && !mBoxObjectTable->Init(12)) {
       mBoxObjectTable = nsnull;
     }

content/base/src/nsDocument.h

@@ -776,11 +776,13 @@ protected:
   // True if document has ever had script handling object.
   PRPackedBool mHasHadScriptHandlingObject:1;
 
+  PRPackedBool mHasWarnedAboutBoxObjects:1;
+
   PRUint8 mXMLDeclarationBits;
 
   PRUint8 mDefaultElementType;
 
-  nsInterfaceHashtable<nsISupportsHashKey, nsPIBoxObject> *mBoxObjectTable;
+  nsInterfaceHashtable<nsVoidPtrHashKey, nsPIBoxObject> *mBoxObjectTable;
   nsInterfaceHashtable<nsVoidPtrHashKey, nsISupports> *mContentWrapperHash;
 
   // The channel that got passed to StartDocumentLoad(), if any

content/base/src/nsGenericElement.cpp

@@ -2123,8 +2123,6 @@ nsGenericElement::UnbindFromTree(PRBool aDeep, PRBool aNullParent)
     if (HasAttr(kNameSpaceID_XLink, nsGkAtoms::href)) {
       document->ForgetLink(this);
     }
-
-    document->ClearBoxObjectFor(this);
   }
 
   // Unset things in the reverse order from how we set them in BindToTree
@@ -2891,7 +2889,6 @@ nsGenericElement::DestroyContent()
   nsIDocument *document = GetOwnerDoc();
   if (document) {
     document->BindingManager()->ChangeDocumentFor(this, document, nsnull);
-    document->ClearBoxObjectFor(this);
   }
 
   PRUint32 i, count = mAttrsAndChildren.ChildCount();

content/base/src/nsNodeUtils.cpp

@@ -235,6 +235,13 @@ nsNodeUtils::LastRelease(nsINode* aNode)
     aNode->UnsetFlags(NODE_HAS_LISTENERMANAGER);
   }
 
+  if (aNode->IsNodeOfType(nsINode::eELEMENT)) {
+    nsIDocument* ownerDoc = aNode->GetOwnerDoc();
+    if (ownerDoc) {
+      ownerDoc->ClearBoxObjectFor(static_cast<nsIContent*>(aNode));
+    }
+  }
+
   delete aNode;
 }
 
@@ -544,6 +551,9 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNode, PRBool aClone, PRBool aDeep,
     nsCOMPtr<nsISupports> oldRef;
     nsIDocument* oldDoc = aNode->GetOwnerDoc();
     if (oldDoc) {
+      if (aNode->IsNodeOfType(nsINode::eELEMENT)) {
+        oldDoc->ClearBoxObjectFor(static_cast<nsIContent*>(aNode));
+      }
       oldRef = oldDoc->GetReference(aNode);
       if (oldRef) {
         oldDoc->RemoveReference(aNode);

content/xul/content/src/nsXULElement.cpp

@@ -1830,9 +1830,7 @@ nsXULElement::GetBoxObject(nsIBoxObject** aResult)
   *aResult = nsnull;
 
   // XXX sXBL/XBL2 issue! Owner or current document?
-  nsIDocument* doc = HasFlag(NODE_FORCE_XBL_BINDINGS) ?
-    GetOwnerDoc() : GetCurrentDoc();
-  nsCOMPtr<nsIDOMNSDocument> nsDoc(do_QueryInterface(doc));
+  nsCOMPtr<nsIDOMNSDocument> nsDoc = do_QueryInterface(GetOwnerDoc());
 
   return nsDoc ? nsDoc->GetBoxObjectFor(this, aResult) : NS_ERROR_FAILURE;
 }

dom/locales/en-US/chrome/dom/dom.properties

@@ -55,5 +55,6 @@ UseOfReleaseEventsWarning=Use of releaseEvents() is deprecated, see bug 330494.
 UseOfRouteEventWarning=Use of routeEvent() is deprecated, see bug 330494.
 UseOfPreventBubbleWarning=Event=%S, use of preventBubble() is deprecated. Use W3C standard stopPropagation() instead.
 UseOfPreventCaptureWarning=Event=%S, use of preventCapture() is deprecated. Use W3C standard stopPropagation() instead.
+UseOfGetBoxObjectForWarning=Use of getBoxObjectFor() is deprecated. Try to use element.getBoundingClientRect() if possible.
 UnexpectedCanvasVariantStyle=canvas: an attempt to set strokeStyle or fillStyle to a value that is neither a string, a CanvasGradient, or a CanvasPattern was ignored.
 EmptyGetElementByIdParam=Empty string passed to getElementById().