Closed
Bug 564266
Opened 14 years ago
Closed 14 years ago
DOMCI GetItemAt/GetNamedItem should return nsWrapperCache
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla2.0b3
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: peterv, Assigned: peterv)
References
(Blocks 1 open bug)
Details
Attachments
(6 files)
16.45 KB,
patch
|
jst
:
review+
|
Details | Diff | Splinter Review |
34.92 KB,
patch
|
jst
:
review+
|
Details | Diff | Splinter Review |
26.75 KB,
patch
|
jst
:
review+
|
Details | Diff | Splinter Review |
23.44 KB,
patch
|
jst
:
review+
|
Details | Diff | Splinter Review |
27.13 KB,
patch
|
jst
:
review+
|
Details | Diff | Splinter Review |
22.50 KB,
patch
|
jst
:
review+
|
Details | Diff | Splinter Review |
That way we can avoid QI'ing for it from WrapNative.
Assignee | ||
Comment 1•14 years ago
|
||
I have a series of patches for this already, will try to attach soon.
Assignee | ||
Comment 2•14 years ago
|
||
Assignee | ||
Comment 3•14 years ago
|
||
Assignee | ||
Comment 4•14 years ago
|
||
Assignee | ||
Comment 5•14 years ago
|
||
Assignee | ||
Comment 6•14 years ago
|
||
Assignee | ||
Comment 7•14 years ago
|
||
Assignee | ||
Updated•14 years ago
|
Attachment #451926 -
Flags: review?(jst)
Assignee | ||
Updated•14 years ago
|
Attachment #451928 -
Flags: review?(jst)
Updated•14 years ago
|
Attachment #451926 -
Flags: review?(jst) → review+
Comment 8•14 years ago
|
||
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!
Assignee | ||
Comment 10•14 years ago
|
||
If it works that's fine by me.
Assignee | ||
Updated•14 years ago
|
Attachment #451930 -
Flags: review?(jst)
Assignee | ||
Updated•14 years ago
|
Attachment #451932 -
Flags: review?(jst)
Assignee | ||
Updated•14 years ago
|
Attachment #451933 -
Flags: review?(jst)
Assignee | ||
Updated•14 years ago
|
Attachment #451939 -
Flags: review?(jst)
Updated•14 years ago
|
Attachment #451930 -
Flags: review?(jst) → review+
Updated•14 years ago
|
blocking2.0: --- → betaN+
Updated•14 years ago
|
Attachment #451932 -
Flags: review?(jst) → review+
Comment 11•14 years ago
|
||
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+
Updated•14 years ago
|
Attachment #451939 -
Flags: review?(jst) → review+
Assignee | ||
Comment 12•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/22a1af92178a http://hg.mozilla.org/mozilla-central/rev/4990659913a6 http://hg.mozilla.org/mozilla-central/rev/3fd6c91fa48b http://hg.mozilla.org/mozilla-central/rev/ad9bb5ef8cf1
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b3
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•