Bug 42021 – isEqualNode should work for DocumentType nodes

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Sam Weinig]

isEqualNode is implemented incorrectly for DocumentType nodes.  According to DOM3, it supposed to check the publicId, systemId, internalSubset, entities and notations are equal.

[Sam Weinig]

Created attachment 61143 [details]
Patch

[Andreas Kling]

Comment on attachment 61143 [details]
Patch

>WebCore/dom/Node.cpp:1795
> +          NamedNodeMap* othereEntities = documentTypeOther->entities();
Variable name should be 'otherEntities'

>WebCore/dom/Node.cpp:1798
> +          if (entities && entities->mapsEquivalent(othereEntities))
> +              return false;
This looks wrong. Shouldn't it be "if (entities && !entities->mapsEquivalent(othereEntities))"?

Same two comments for the 'notations' section below.

[Sam Weinig]

(In reply to comment #2)
> (From update of attachment 61143 [details])
> >WebCore/dom/Node.cpp:1795
> > +          NamedNodeMap* othereEntities = documentTypeOther->entities();
> Variable name should be 'otherEntities'
> 
> >WebCore/dom/Node.cpp:1798
> > +          if (entities && entities->mapsEquivalent(othereEntities))
> > +              return false;
> This looks wrong. Shouldn't it be "if (entities && !entities->mapsEquivalent(othereEntities))"?
> 
> Same two comments for the 'notations' section below.

Thanks, I made these changes locally. Unfortunately these two bits are not testable at the moment.

[Sam Weinig]

Landed in http://trac.webkit.org/changeset/63051.

[Darin Adler]

Comment on attachment 61143 [details]
Patch

Typos: othereEntities and othereNotations. Does mapsEquavalent handle an argument of 0 and return false? Those test cases don't seem to cover all the branches in that function.

[Sam Weinig]

(In reply to comment #5)
> (From update of attachment 61143 [details])
> Typos: othereEntities and othereNotations. Does mapsEquavalent handle an argument of 0 and return false? Those test cases don't seem to cover all the branches in that function.

mapsEquavalent does handle a null map.  I can't currently test the entities, notations or internalSubset branches as those can't currently be set to anything other than the default (null or the null string) in WebCore.  I left in the tests since this is not a hot function but I could remove them if you think that would be better.

[Alexey Proskuryakov]

*** Bug 38786 has been marked as a duplicate of this bug. ***

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.