[webkit-reviews] review denied: [Bug 53031] isContentEditable is not working properly with document.designMode : [Attachment 81715] fix patch 3

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Chang Shu <Chang.Shu at nokia.com>'s
request for review:
Bug 53031: isContentEditable is not working properly with document.designMode
https://bugs.webkit.org/show_bug.cgi?id=53031

Attachment 81715: fix patch 3
https://bugs.webkit.org/attachment.cgi?id=81715&action=review

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

Could you also test a case where you set contenteditable programmatically? 
(i.e. div.contenteditable = false / true).  r- for style issues.

>
LayoutTests/fast/dom/HTMLElement/iscontenteditable-designmodeon-allinherit.html
:19
> +<div id="div1">
> +    <div>
> +	   <div id="div"></div>
> +    </div>
> +</div>
> +<div id="div2">
> +    <div>
> +	   <p id="p"></p>
> +    </div>

Why do we need div1/div2 and empty divs?  id's div1 and div2 can be eliminated
since we're not referencing them anywhere.  Please simplify the markup so that
it doesn't clutter the test needlessly.

>
LayoutTests/fast/dom/HTMLElement/iscontenteditable-designmodeon-ancestor.html:1
1
> +<div id="div1" contentEditable="false">

div1 is not a descriptive name.  How about parent_of_div?

>
LayoutTests/fast/dom/HTMLElement/iscontenteditable-designmodeon-ancestor.html:1
6
> +<div id="div2" contentEditable="true">

Ditto. parent_of_p?