[webkit-reviews] review granted: [Bug 89171] [Crash][Editing] pressing 'enter' on LI element triggers assertion in Shadow DOM : [Attachment 148603] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Shinya Kawanaka
<shinyak at chromium.org>'s request for review:
Bug 89171: [Crash][Editing] pressing 'enter' on LI element triggers assertion
in Shadow DOM
https://bugs.webkit.org/show_bug.cgi?id=89171

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

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=148603&action=review


> Source/WebCore/ChangeLog:8
> +	   Since modidying Shadow DOM removes renderer of elements in Shadow
DOM and Shadow host,

Why is "shadow" capitalized?

> Source/WebCore/editing/CompositeEditCommand.cpp:837
> +    // container->renderer() might be gone unless updating style in Shadow
DOM.

I don't think this comment is necessary. It is understood that updating layout
helps with that.

> LayoutTests/editing/shadow/pressing-enter-on-list.html:18
> +<pre id="console"></pre>

We don't need console, do we?

> LayoutTests/editing/shadow/pressing-enter-on-list.html:34
> +// Pressing enter should not trigger assertion.

This comment is redundant because the description clearly states that.