[Webkit-unassigned] [Bug 78473] ShadowRoot needs innerHTML

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=78473





--- Comment #11 from Roland Steiner <rolandsteiner at chromium.org>  2012-02-22 03:45:28 PST ---
(From update of attachment 127944)
View in context: https://bugs.webkit.org/attachment.cgi?id=127944&action=review

>>> Source/WebCore/dom/ShadowRoot.cpp:157
>>> +        replaceChildrenWithFragment(toElement(this), fragment.release(), ec);
>> 
>> Does this really work?
> 
> I am assuming this works as DocumentFragment is derived from ContainerNode.

Even so this is beyond fragile. It only happens to work because currently replaceChildrenWithFragment uses ContainerNode methods only. I suggest making the parameter a ContainerNode* and do away with toElement(). Then any changes to to replaceChildrenWithFragment will break at compile time rather than runtime.

> Source/WebCore/editing/markup.cpp:940
> +PassRefPtr<DocumentFragment> createFragmentFromSource(const String& markup, Element* element, ExceptionCode& ec)

editing/ doesn't seem the right place to me for these methods. IMHO they'd better be moved to dom/ - either into ContainerNode or, better yet, into a separate file.

>>> LayoutTests/fast/dom/shadow/shadow-root-innerHTML.html:36
>>> +
>> 
>> I'm confused. Why is div2 available even after setting another innerHTML?
> 
> Its only to show that it resides in the dom tree of the ShadowRoot. After setting innerHTML, the children gets replaced with the fragment.

I have to say I'm likewise confused - as you say, it should get replaced and no longer be in the tree. So why is it still accessible via getElementById?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.