[webkit-reviews] review denied: [Bug 78473] ShadowRoot needs innerHTML : [Attachment 127944] Updated Patch

[bugzilla-daemon at webkit.org]

MORITA Hajime <morrita at google.com> has denied Kaustubh Atrawalkar
<kaustubh at motorola.com>'s request for review:
Bug 78473: ShadowRoot needs innerHTML
https://bugs.webkit.org/show_bug.cgi?id=78473

Attachment 127944: Updated Patch
https://bugs.webkit.org/attachment.cgi?id=127944&action=review

------- Additional Comments from MORITA Hajime <morrita at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=127944&action=review


>> 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.

I think this is OK for this time if we have a bug to move this to appropriate
place.

>>>> 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?

I tried this locally and it triggered an assertion failure on the debug build.