Summary: | isEqualNode should work for DocumentType nodes | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sam Weinig <sam> | ||||
Component: | DOM | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | cdumez, jamesr, kling | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | OS X 10.5 | ||||||
Attachments: |
|
Description
Sam Weinig
2010-07-10 00:17:55 PDT
Created attachment 61143 [details]
Patch
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. (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. Landed in http://trac.webkit.org/changeset/63051. 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.
(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. *** Bug 38786 has been marked as a duplicate of this bug. *** Mass moving XML DOM bugs to the "DOM" Component. |