[webkit-reviews] review denied: [Bug 114960] Wrong text position when you click backspace on the line below the image : [Attachment 225874] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Sudarshan C P
<sudarshan.cp at samsung.com>'s request for review:
Bug 114960: Wrong text position when you click backspace on the line below the
image
https://bugs.webkit.org/show_bug.cgi?id=114960

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

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


> Source/WebCore/ChangeLog:11
> +	   While finding new position for inserting element after
deleting/pasting,
> +	   included image element also for find the proper position to merge
the split node.

This description should be appear before "Test:" line but after "Reviewed by"
line.
But this doesn't really describe what was causing the bug and how we're fixing
it.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:119
>      // If we're already on a break, it's probably a placeholder and we
shouldn't change our position.
> -    if (editingIgnoresContent(pos.deprecatedNode()))
> +    if (editingIgnoresContent(pos.deprecatedNode()) &&
!pos.deprecatedNode()->hasTagName(imgTag))

The problem here is that editingIgnoresContent returns true on more than just a
placeholder.
We should be checking whether the deprecatedNode is br or not instead.
What's weird is that lineBreakExistsAtPosition below should already be checking
this condition.

r- because special-casing img can't be the right fix. e.g. it'll fail for other
elements such as object and hr that are ignored by editing.