[webkit-reviews] review denied: [Bug 25533] Elements on the same line should be treated as such by caret navigation : [Attachment 84236] Patch proposal + Layout test

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Mario Sanchez Prada
<msanchez at igalia.com>'s request for review:
Bug 25533: Elements on the same line should be treated as such by caret
navigation
https://bugs.webkit.org/show_bug.cgi?id=25533

Attachment 84236: Patch proposal + Layout test
https://bugs.webkit.org/attachment.cgi?id=84236&action=review

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

> Source/WebCore/ChangeLog:28
> +	   (WebCore::createCommandMap): Add the new commands needed for the
> +	   GTK port, related with up/down movement in caret browsing mode.

I don't think it's correct to fix just GTK port.  This is a bug that needs to
be fixed on all ports regardless of whether the caret navigation is enabled or
not.  To demonstrate the bug on other ports, simply enable the design mode on
the attached test case.

> Source/WebCore/editing/EditorCommand.cpp:620
> +static bool isVerticallyCoincident(VisiblePosition origin, VisiblePosition
candidate, int originX, SelectionDirection direction)
> +{

This is definitely not right.  We shouldn't be adding these selection related
code in EditorCommand.cpp, which is supposed to be delegating all the work to
the rest of editing code.