Closed Bug 564266 Opened 10 years ago Closed 9 years ago

DOMCI GetItemAt/GetNamedItem should return nsWrapperCache

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b3
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: peterv, Assigned: peterv)

References

(Blocks 1 open bug)

Details

Attachments

(6 files)

That way we can avoid QI'ing for it from WrapNative.
I have a series of patches for this already, will try to attach soon.
Attachment #451926 - Flags: review?(jst)
Attachment #451928 - Flags: review?(jst)
Attachment #451926 - Flags: review?(jst) → review+
Comment on attachment 451928 [details] [diff] [review]
Part 2 (store nsHTMLOptionElement in nsHTMLOptionCollection) v1

- In nsFormContentList::nsFormContentList():

   for (i = 0; i < length; i++) {
-    aContentList.Item(i, getter_AddRefs(item));
-
-    nsCOMPtr<nsIContent> c(do_QueryInterface(item));
-
+    nsIContent *c = aContentList.GetNodeAt(0);

You want aContentList.GetNodeAt(i) here don't you?

r=jst
Attachment #451928 - Flags: review?(jst) → review+
peterv,

121   nsHTMLOptionElement *ItemAsOption(PRUint32 aIndex)
122   {
123     return mElements.SafeElementAt(aIndex, nsRefPtr<nsHTMLOptionElement>());
124   }

Can we use
mElements.SafeElementAt(aIndex, nsnull);
here?

Is there any difference?

I'm asking because this line doesn't compile on Solaris with Sun Studio.
Error: Overloading ambiguity between "nsTArray<nsRefPtr<nsHTMLOptionElement>>::SafeElementAt(unsigned, nsRefPtr<nsHTMLOptionElement>&)" and "nsTArray<nsRefPtr<nsHTMLOptionElement>>::SafeElementAt(unsigned, const nsRefPtr<nsHTMLOptionElement>&) const".

Thanks!
If it works that's fine by me.
Attachment #451930 - Flags: review?(jst)
Attachment #451932 - Flags: review?(jst)
Attachment #451933 - Flags: review?(jst)
Attachment #451939 - Flags: review?(jst)
Attachment #451930 - Flags: review?(jst) → review+
blocking2.0: --- → betaN+
Attachment #451932 - Flags: review?(jst) → review+
Comment on attachment 451933 [details] [diff] [review]
Part 5 (make ResolveName and GetDocumentAllResult return the wrapper cache) v1

- In nsHTMLDocument::GetDocumentAllResult():

   Element* root = GetRootElement();
   if (!root) {
-    return NS_OK;
+    *aResult = NS_OK;
+
+    return nsnull;
   }
 
   nsRefPtr<nsContentList> docAllList = entry->GetDocAllList();
   if (!docAllList) {
     docAllList = new nsContentList(root, DocAllResultMatch,
                                    nsnull, nsnull, PR_TRUE, id);
-    NS_ENSURE_TRUE(docAllList, NS_ERROR_OUT_OF_MEMORY);
     entry->SetDocAllList(docAllList);
   }
 
+  *aResult = NS_OK;
+

Maybe just move this up above the call to GetrootElement() and avoid having to set *aResult in the if (!root) case as well?

- In nsWindowSH::GlobalScopePolluterNewResolve():

+  nsRefPtr<nsHTMLDocument> document = GetDocument(cx, obj);

The document should be held alive by obj here, so no need for document to be a strong pointer AFAICT.

r=jst with that considered.
Attachment #451933 - Flags: review?(jst) → review+
Attachment #451939 - Flags: review?(jst) → review+
Depends on: 580969
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.