Bug 196466

Summary: Attr nodes are not cloned properly
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: DOMAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, dbates, esprehn+autocc, ews-watchlist, ggaren, kangil.han, koivisto, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://dom.spec.whatwg.org/#concept-node-clone
Attachments:
Description Flags
Patch rniwa: review+

Description Chris Dumez 2019-04-01 15:09:45 PDT 
Attr nodes are not cloned properly according to:
- https://dom.spec.whatwg.org/#concept-node-clone

A cloned Attr node should retain its prefix and namespace.

Both Gecko and Blink agree with the DOM specification here.
Comment 1 Chris Dumez 2019-04-01 15:11:57 PDT 
Created attachment 366428 [details]
Patch
Comment 2 Ryosuke Niwa 2019-04-01 18:52:03 PDT 
Comment on attachment 366428 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=366428&action=review

> Source/WebCore/dom/Document.cpp:992
>          // FIXME: This will "Attr::normalize" child nodes of Attr.

Should we remove this FIXME?
Comment 3 Chris Dumez 2019-04-01 19:40:35 PDT 
Comment on attachment 366428 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=366428&action=review

>> Source/WebCore/dom/Document.cpp:992
>>          // FIXME: This will "Attr::normalize" child nodes of Attr.
> 
> Should we remove this FIXME?

Is it not valid anymore?
Comment 4 Ryosuke Niwa 2019-04-01 19:47:17 PDT 
(In reply to Chris Dumez from comment #3)
> Comment on attachment 366428 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=366428&action=review
> 
> >> Source/WebCore/dom/Document.cpp:992
> >>          // FIXME: This will "Attr::normalize" child nodes of Attr.
> > 
> > Should we remove this FIXME?
> 
> Is it not valid anymore?

There is no Attr::normalize anymore. This is about normalizing the text child nodes of Attr when we supported multiple child nodes for Attr:
https://dom.spec.whatwg.org/#dom-node-normalize

But that's totally irrelevant now since Attr isn't even a ContainerNode now.
Comment 5 Chris Dumez 2019-04-01 20:14:56 PDT 
Committed r243734: <https://trac.webkit.org/changeset/243734>
Comment 6 Radar WebKit Bug Importer 2019-04-01 20:15:18 PDT 
<rdar://problem/49506786>
Comment 7 Chris Dumez 2019-04-01 20:15:31 PDT 
(In reply to Ryosuke Niwa from comment #4)
> (In reply to Chris Dumez from comment #3)
> > Comment on attachment 366428 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=366428&action=review
> > 
> > >> Source/WebCore/dom/Document.cpp:992
> > >>          // FIXME: This will "Attr::normalize" child nodes of Attr.
> > > 
> > > Should we remove this FIXME?
> > 
> > Is it not valid anymore?
> 
> There is no Attr::normalize anymore. This is about normalizing the text
> child nodes of Attr when we supported multiple child nodes for Attr:
> https://dom.spec.whatwg.org/#dom-node-normalize
> 
> But that's totally irrelevant now since Attr isn't even a ContainerNode now.

Oh right. Fixed before landing, thanks.