[Webkit-unassigned] [Bug 25533] Elements on the same line should be treated as such by caret navigation

[bugzilla-daemon at webkit.org]

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





--- Comment #15 from Mario Sanchez Prada <msanchez at igalia.com>  2011-03-02 02:36:34 PST ---
(In reply to comment #14)
> (From update of attachment 84236 [details])
> 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. 

Actually the code I've introduced is common to all the platforms, it's just that the GTK port is the only one that would use it at the moment, since it's the only one, afaik, with caret browsing mode.

I created those 4 new commands precisely because modifying the behavior of the already present MoveUp/MoveDown commands would have an impact in the current API, and that seemed to me like a bad idea.

> This is a bug that needs to be fixed on all ports regardless of whether the
> caret navigation is enabled or not.

I checked what Chrome and Safari does when moving across a table (not in caret browsing mode, but selecting a single character and then extending the selection with shift+arrows) and I found they traverse tables in the same (wrong?) way: cell by cell in a row by row basis.

Obviously, my current patch wouldn't fix the behavior for those other ports since I'm defining four new commands precisely not to touch current up/down behavior, but your comment makes me think perhaps I should actually be modifying those current commands, instead of creating new ones.

Could you please clarify?

> To demonstrate the bug on other ports, simply enable the design mode on the
> attached test case.

What do you mean by "enable the design mode"?

> > 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.

Could you provide some guidance, even roughly, about how this could be done then? I'm not fond on this so deep issues in editing stuff and probably I'm missing something :-)

Thanks for the great feedback

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