Adjust GetLeft/RightSelectionModel return, cursor on edit.

Rename GetLeft/RightCursorPosition to GetLeft/RightSelectionModel.
Discard selection from the the GetLeft/RightSelectionModel return value.
Update the cursor properly on edits.


BUG=90426
TEST=--use-pure-views cursor movement, position after editing.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=95945

[msw, 2011-08-08 22:21:25]

PTAL, thanks!

[xji, 2011-08-08 23:56:13]

http://codereview.chromium.org/7598014/diff/3001/ui/gfx/render_text.cc
File ui/gfx/render_text.cc (right):

http://codereview.chromium.org/7598014/diff/3001/ui/gfx/render_text.cc#newcod...
ui/gfx/render_text.cc:186: sel.set_caret_placement(SelectionModel::TRAILING);
RenderText::ClearSelection() probably wont work after the change?

http://codereview.chromium.org/7598014/diff/3001/ui/gfx/render_text.cc#newcod...
ui/gfx/render_text.cc:192: GetCursorPosition(), SelectionModel::LEADING);
should the position be selection_model_?
or selection_model_ except selection_start set to selection_end?

Moving the logic of LINE_BREAK inside GetLeftCursorPosition() seems make the
code cleaner.
but does it make sense to have the LINE_BREAK code go through the selection
bounds leftness checking?

http://codereview.chromium.org/7598014/diff/3001/ui/gfx/render_text.cc#newcod...
ui/gfx/render_text.cc:214: GetCursorPosition(), SelectionModel::LEADING);
ditto

http://codereview.chromium.org/7598014/diff/3001/ui/gfx/render_text.cc#newcod...
ui/gfx/render_text.cc:535: SelectionModel::LEADING);
this will return the rightmost visual extreme of the line. which is not in
parity with GetLeftSelectionModel(...LINE_BREAK).
the parity one should be: 
SelectionModel(text().length(), GetPreviousGrapheme(),
SelectionModel::TRAILING);

http://codereview.chromium.org/7598014/diff/3001/ui/gfx/render_text.h
File ui/gfx/render_text.h (right):

http://codereview.chromium.org/7598014/diff/3001/ui/gfx/render_text.h#newcode274
ui/gfx/render_text.h:274: virtual size_t GetPreviousGrapheme(size_t position)
const;
Maybe GetIndexOfPreviousGrapheme or IndexOfPreviousGrapheme?

[msw, 2011-08-09 01:07:42]

Comments addressed; PTAL, thanks!

http://codereview.chromium.org/7598014/diff/3001/ui/gfx/render_text.cc
File ui/gfx/render_text.cc (right):

http://codereview.chromium.org/7598014/diff/3001/ui/gfx/render_text.cc#newcod...
ui/gfx/render_text.cc:186: sel.set_caret_placement(SelectionModel::TRAILING);
On 2011/08/08 23:56:13, xji wrote:
> RenderText::ClearSelection() probably wont work after the change?
> 

Done.

http://codereview.chromium.org/7598014/diff/3001/ui/gfx/render_text.cc#newcod...
ui/gfx/render_text.cc:192: GetCursorPosition(), SelectionModel::LEADING);
On 2011/08/08 23:56:13, xji wrote:
> should the position be selection_model_?
> or selection_model_ except selection_start set to selection_end?
> 
> Moving the logic of LINE_BREAK inside GetLeftCursorPosition() seems make the
> code cleaner.
> but does it make sense to have the LINE_BREAK code go through the selection
> bounds leftness checking? 

Done.

http://codereview.chromium.org/7598014/diff/3001/ui/gfx/render_text.cc#newcod...
ui/gfx/render_text.cc:214: GetCursorPosition(), SelectionModel::LEADING);
On 2011/08/08 23:56:13, xji wrote:
> ditto

Done.

http://codereview.chromium.org/7598014/diff/3001/ui/gfx/render_text.cc#newcod...
ui/gfx/render_text.cc:535: SelectionModel::LEADING);
On 2011/08/08 23:56:13, xji wrote:
> this will return the rightmost visual extreme of the line. which is not in
> parity with GetLeftSelectionModel(...LINE_BREAK).
> the parity one should be: 
> SelectionModel(text().length(), GetPreviousGrapheme(),
> SelectionModel::TRAILING);
> 

Done.

http://codereview.chromium.org/7598014/diff/3001/ui/gfx/render_text.h
File ui/gfx/render_text.h (right):

http://codereview.chromium.org/7598014/diff/3001/ui/gfx/render_text.h#newcode274
ui/gfx/render_text.h:274: virtual size_t GetPreviousGrapheme(size_t position)
const;
On 2011/08/08 23:56:13, xji wrote:
> Maybe GetIndexOfPreviousGrapheme or IndexOfPreviousGrapheme? 

Done.

[xji, 2011-08-09 01:13:19]

lgtm