Bug 42021 - isEqualNode should work for DocumentType nodes
Summary: isEqualNode should work for DocumentType nodes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 38786 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-07-10 00:17 PDT by Sam Weinig
Modified: 2019-02-06 09:04 PST (History)
3 users (show)

See Also:


Attachments
Patch (6.77 KB, patch)
2010-07-10 00:19 PDT, Sam Weinig
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2010-07-10 00:17:55 PDT
isEqualNode is implemented incorrectly for DocumentType nodes.  According to DOM3, it supposed to check the publicId, systemId, internalSubset, entities and notations are equal.
Comment 1 Sam Weinig 2010-07-10 00:19:21 PDT
Created attachment 61143 [details]
Patch
Comment 2 Andreas Kling 2010-07-10 08:10:50 PDT
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.
Comment 3 Sam Weinig 2010-07-10 14:27:05 PDT
(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.
Comment 4 Sam Weinig 2010-07-10 14:39:01 PDT
Landed in http://trac.webkit.org/changeset/63051.
Comment 5 Darin Adler 2010-07-10 15:19:09 PDT
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.
Comment 6 Sam Weinig 2010-07-10 15:29:13 PDT
(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.
Comment 7 Alexey Proskuryakov 2010-07-11 09:12:27 PDT
*** Bug 38786 has been marked as a duplicate of this bug. ***
Comment 8 Lucas Forschler 2019-02-06 09:04:03 PST
Mass moving XML DOM bugs to the "DOM" Component.