[Webkit-unassigned] [Bug 33950] contentEditable with "position:relative" paragraphs is buggy

[bugzilla-daemon at webkit.org]

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





--- Comment #37 from Ryosuke Niwa <rniwa at webkit.org>  2013-10-09 13:00:58 PST ---
(From update of attachment 213794)
View in context: https://bugs.webkit.org/attachment.cgi?id=213794&action=review

> LayoutTests/ChangeLog:8
> +        LayoutTest:

Don't this need line.

> LayoutTests/ChangeLog:12
> +        And expected output is updated with correct behaviour.

We call that "rebaseline". So just say "Rebaselined some tests" but you should explain why new outputs are correct.

> LayoutTests/ChangeLog:16
> +        * editing/execCommand/positioned-no-special-element-expected.txt: Added.
> +        * editing/execCommand/positioned-no-special-element.html: Added.

I would have moved this test into editing/deleting and named it deleting-relatively-positioned-special-element.html to be more precise.

> LayoutTests/ChangeLog:20
> +        * platform/mac/editing/unsupported-content/table-delete-001-expected.txt:
> +        * platform/mac/editing/unsupported-content/table-delete-003-expected.txt:

Why don't we convert these tests to dump-as-markup/marku-as-text tests first?
The change in the expected result is hard to understand as is.

> LayoutTests/editing/execCommand/positioned-no-special-element.html:9
> +    <div id="container"  contenteditable><p>1</p><p id="paraToDelete">2</p><p>3</p><p>4</p></div>

Please spell out paragraph. Also there are two spaces before contenteditable.

> LayoutTests/editing/execCommand/positioned-no-special-element.html:12
> +<script>

Why is this script element defined outside of body?

> LayoutTests/editing/execCommand/positioned-no-special-element.html:13
> +    window.getSelection().setPosition(paraToDelete, 1);

We don't need "window.".  We should use the standard collapse function as in:
getSelection().collapse(paragraphToDelete, 1);

> LayoutTests/editing/execCommand/positioned-no-special-element.html:18
> +    Markup.dump('container');

You can call dump after setting the selection but before calling execCommand('Delete') to show the initial state.
That'll make the test result even more self-descriptive.

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