Issue 7607018: Remove PREVIOUS_GRAPHEME_TRAILING.

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 7607018: Remove PREVIOUS_GRAPHEME_TRAILING. (Closed)

Created:
9 years, 4 months ago by xji

Modified:
9 years, 4 months ago

Reviewers:
msw

CC:
chromium-reviews, Paweł Hajdan Jr., dhollowa

Base URL:
svn://svn.chromium.org/chrome/trunk/src/

Visibility:
Public.

Description

Remove PREVIOUS_GRAPHEME_TRAILING. Remove const from GetIndexOfPreviousGrapheme() so the override function could update local data. Construct SelectionModel correctly in unittest. BUG=90426 TEST=--use-pure-view. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=97029

Patch Set 1 #

Total comments: 8

Patch Set 2 : '' #

Created: 9 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+26 lines, -52 lines)			Patch
M	ui/gfx/render_text.h	View	1	2 chunks	+2 lines, -4 lines	0 comments	Download
M	ui/gfx/render_text.cc	View	1	4 chunks	+11 lines, -4 lines	0 comments	Download
M	views/controls/textfield/textfield_views_model.cc	View	1	2 chunks	+2 lines, -4 lines	0 comments	Download
M	views/controls/textfield/textfield_views_model_unittest.cc	View	1	5 chunks	+11 lines, -40 lines	0 comments	Download

Messages

Total messages: 1 (0 generated)

Expand Messages | Collapse Messages

msw

9 years, 4 months ago (2011-08-13 19:37:38 UTC) #1

LGTM with some cleanup.

http://codereview.chromium.org/7607018/diff/1/views/controls/textfield/textfi...
File views/controls/textfield/textfield_views_model_unittest.cc (right):

http://codereview.chromium.org/7607018/diff/1/views/controls/textfield/textfi...
views/controls/textfield/textfield_views_model_unittest.cc:129:
gfx::SelectionModel selection(1U);
These two lines are no longer necessary.

http://codereview.chromium.org/7607018/diff/1/views/controls/textfield/textfi...
views/controls/textfield/textfield_views_model_unittest.cc:131: selection =
gfx::SelectionModel(1U, 3U);
You can optionally get rid of |selection| and just construct SelectionModels in
the calls to MoveCursorTo.

http://codereview.chromium.org/7607018/diff/1/views/controls/textfield/textfi...
views/controls/textfield/textfield_views_model_unittest.cc:808:
gfx::SelectionModel sel(1);
You can remove these two lines.

http://codereview.chromium.org/7607018/diff/1/views/controls/textfield/textfi...
views/controls/textfield/textfield_views_model_unittest.cc:897:
sel.set_selection_end(1);
You can remove these three lines.

http://codereview.chromium.org/7607018/diff/1/views/controls/textfield/textfi...
views/controls/textfield/textfield_views_model_unittest.cc:1025:
gfx::SelectionModel sel(1);
You can remove these two lines.

http://codereview.chromium.org/7607018/diff/1/views/controls/textfield/textfi...
views/controls/textfield/textfield_views_model_unittest.cc:1035:
gfx::SelectionModel sel(3);
You can remove these two lines.

http://codereview.chromium.org/7607018/diff/1/views/controls/textfield/textfi...
views/controls/textfield/textfield_views_model_unittest.cc:1045:
gfx::SelectionModel sel(1);
You can remove these two lines.

http://codereview.chromium.org/7607018/diff/1/views/controls/textfield/textfi...
views/controls/textfield/textfield_views_model_unittest.cc:1055:
gfx::SelectionModel sel(3);
You can remove these two lines.

Expand Messages | Collapse Messages