[webkit-reviews] review denied: [Bug 34608] Deleting text leaves the style of text in the typing style : [Attachment 146379] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Pravin D <pravind.2k4 at gmail.com>'s
request for review:
Bug 34608: Deleting text leaves the style of text in the typing style
https://bugs.webkit.org/show_bug.cgi?id=34608

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

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


> Source/WebCore/editing/DeleteSelectionCommand.cpp:493
> +		       // Remove any style belonging to this node from typing
style.
> +		       m_typingStyle->removeStyleAddedByNode(node.get());

This is very inefficient. A better way of fixin this bug is to set the typing
style at the end of the whole deletion.

> LayoutTests/editing/deleting/delete-reset-typing-style.html:1
> +<title> Test case for bug
https://bugs.webkit.org/show_bug.cgi?id=34608</title>

No DOCTYPE? no html? and no head?
Why does this test need to use quirks mode?

> LayoutTests/editing/deleting/delete-reset-typing-style.html:26
> +function Deletebold(){
> +var test = document.getElementById('bold');
> +window.getSelection().setPosition(test, test.innerHTML.length);
> +for (var i = 0; i < 11; i++)
> +    document.execCommand('Delete', false, null);
> +document.execCommand('InsertText', false, 'This should not be BOLD');
> +}

Please put a space after (). and indent the function definition.
Also use CamelCase or camelCase for the function name.