[webkit-reviews] review denied: [Bug 70277] Assert in MoveSelectionCommand::doApply : [Attachment 113037] Proposed patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Devdatta <pwjd73 at motorola.com>'s
request for review:
Bug 70277: Assert in MoveSelectionCommand::doApply
https://bugs.webkit.org/show_bug.cgi?id=70277

Attachment 113037: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=113037&action=review

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


I'd say r- due to various nits.

> LayoutTests/fast/events/drag-link.html:5
> +    function dragElement(srcElement, destElement) {

Indenting scripts like this seems unnecessary.

> LayoutTests/fast/events/drag-link.html:7
> +				   srcElement.offsetTop +
srcElement.offsetHeight / 2);

Wrong indentation. It should be 
	eventSender.mouseMoveTo(srcElement.offsetLeft + srcElement.offsetWidth
/ 2,
	    srcElement.offsetTop + srcElement.offsetHeight / 2);

> LayoutTests/fast/events/drag-link.html:11
> +	   eventSender.mouseMoveTo(destElement.offsetLeft +
destElement.offsetWidth / 2,
> +				   destElement.offsetTop +
destElement.offsetHeight / 2);

Ditto.

> LayoutTests/fast/events/drag-link.html:17
> +    window.onload = function() {
> +	   if (!window.layoutTestController)
> +	       return;

Do we really need to wait until the parsing has ended? Can't we just move the
script element belog #result and run this function directly there?

> LayoutTests/fast/events/drag-link.html:29
> +<div>

Please explain what kind of test this is. In particular, some platform doesn't
implement all features of eventSender, so it's crucial that you explain that
this test is about dragging text not being enabled when selection is not a
range.

> LayoutTests/ChangeLog:11
> +	   Test to drag-drop anchor element on an already focused editable div
> +	   element.

Please add this description after Reviewed by (followed by a blank line) before
the list of files being added (followed by a blank line).