Closed Bug 296450 Opened 15 years ago Closed 15 years ago

Object Dom Node does not work anymore

Categories

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

x86
All
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: Peter6, Assigned: sicking)

References

Details

(Keywords: regression, testcase)

Attachments

(2 files, 1 obsolete file)

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050602
Firefox/1.0+ ID:2005060213

1.open testcase (just to keep it simple)
2.open Domi
3.got to IMG
4.notice Node name in rightscreen shows the title 4 times, no style, alt, src

this regressed between the 
20050601 0740pdt build (works)
and
20050601 0940pdt build (borked)

approximate checkinlist for that period (the checkins during building included)
http://tinderbox.mozilla.org/bonsai/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-06-01+06%3A10%3A00&maxdate=2005-06-01+09%3A40%3A00&cvsroot=%2Fcvsroot

testcase next
Attached file testcase
Flags: blocking-aviary1.1?
*** Bug 296633 has been marked as a duplicate of this bug. ***
My money is on the nsDOMAttributeMap changes from bug 235512.  On the testcase,
the following:

javascript:alert(document.getElementsByTagName("img")[0].attributes.item(2) ==
                 document.getElementsByTagName("img")[0].attributes.item(1))

alerts true...
Assignee: dom-inspector → general
Component: DOM Inspector → DOM
Product: Other Applications → Core
QA Contact: timeless → ian
Could someone confirm that this is still broken in builds that have the fix for
bug 296490?
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050608
Firefox/1.0+

I still see this...
I can't reproduce this on OS X and bz mentioned he couldn't reproduce on Linux :-/.
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050608
Firefox/1.0+ ID:2005060809

This is still broken on Windoze
Attached patch Patch to fix (obsolete) — Splinter Review
I had a patch in my tree that appears to fix this, but it also includes some
other stuff. I think the problem is that the hash in nsDOMAttributeMap passes
around nsAttrKey rather then nsAttrKey&. This at least screws up
nsAttrHashKey::KeyToPointer since you're returning a pointer to the local
variable within that function (this generates a warning which is how I found
it). An alternative fix could have just been to make KeyToPointer take a
KeyType&. This works fine here, but nsTHashtable.h seems to recommend against
it.

The patch also contains the fixe for bug 296207 as well as a fix to make
.ownerDocument work in all situations. I can separate out that stuff if needed.
Assignee: general → bugmail
Status: NEW → ASSIGNED
Attachment #185736 - Flags: superreview?(peterv)
Attachment #185736 - Flags: review?(allan)
Comment on attachment 185736 [details] [diff] [review]
Patch to fix

I'd much rather see this split up, so sr=peterv on these changes (I think I got
them all):

>Index: base/src/nsDOMAttributeMap.cpp
>===================================================================

>@@ -121,13 +139,13 @@ nsDOMAttributeMap::GetAttribute(nsINodeI
> {
>   NS_ASSERTION(aNodeInfo, "GetAttribute() called with aNodeInfo == nsnull!");
>   NS_ASSERTION(aReturn, "GetAttribute() called with aReturn == nsnull");
> 
>   *aReturn = nsnull;
> 
>-  nsAttrHashKey::KeyType attr(aNodeInfo->NamespaceID(), aNodeInfo->NameAtom());
>+  nsAttrKey attr(aNodeInfo->NamespaceID(), aNodeInfo->NameAtom());
> 
>   if (!mAttributeCache.Get(attr, aReturn)) {
>     nsAutoString value;
>     if (aRemove) {
>       // As we are removing the attribute we need to set the current value in
>       // the attribute node.

>Index: base/src/nsDOMAttributeMap.h
>===================================================================

>@@ -76,13 +77,13 @@ public:
> /**
>  * PLDHashEntryHdr implementation for nsAttrKey.
>  */
> class nsAttrHashKey : public PLDHashEntryHdr
> {
> public:
>-  typedef nsAttrKey KeyType;
>+  typedef const nsAttrKey& KeyType;
>   typedef const nsAttrKey* KeyTypePointer;
> 
>   nsAttrHashKey(KeyTypePointer aKey) : mKey(*aKey) {}
>   nsAttrHashKey(const nsAttrHashKey& aCopy) : mKey(aCopy.mKey) {}
>   ~nsAttrHashKey() {}
> 
>@@ -104,13 +105,13 @@ public:
>              (aKey->mNamespaceID << 4) ^
>              NS_PTR_TO_INT32(aKey->mLocalName);
>     }
>   enum { ALLOW_MEMMOVE = PR_TRUE };
> 
> private:
>-  KeyType mKey;
>+  nsAttrKey mKey;
> };
> 
> // Helper class that implements the nsIDOMNamedNodeMap interface.
> class nsDOMAttributeMap : public nsIDOMNamedNodeMap
> {
> public:
Attachment #185736 - Flags: superreview?(peterv) → superreview+
Comment on attachment 185736 [details] [diff] [review]
Patch to fix

(In reply to comment #8)
> I think the problem is that the hash in nsDOMAttributeMap passes
> around nsAttrKey rather then nsAttrKey&. This at least screws up
> nsAttrHashKey::KeyToPointer since you're returning a pointer to the local
> variable within that function (this generates a warning which is how I found
> it).

I cannot reproduce it either, but I can follow the reasoning all right so r+ on
the same stuff that peterv has sr+'ed.
Attachment #185736 - Flags: review?(allan) → review+
Attached patch Separated patchSplinter Review
There were a couple of more blocks, but they're just the same as the parts you
guys reviewed.

Requestiong approval, DOM-Inspector and some aspects of the DOM are broken on
windows without this.
Attachment #185736 - Attachment is obsolete: true
Attachment #185815 - Flags: superreview+
Attachment #185815 - Flags: review+
Attachment #185815 - Flags: approval1.8b3?
Comment on attachment 185815 [details] [diff] [review]
Separated patch

a=caillon for 1.8b3
Attachment #185815 - Flags: approval1.8b3? → approval1.8b3+
Checked in
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Every time when I open the DOM inspector I get two messages in the JS console:

No chrome package registered for chrome://communicator/content/utilityOverlay.xul .
No chrome package registered for chrome://communicator/content/tasksOverlay.xul .
Seems unrelated to this bug
Flags: blocking1.8b3?
Flags: blocking-aviary1.1?
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.