Issue 6318004: Add TextRange and related methods to Textfield Views.

Issue 6318004: Add TextRange and related methods to Textfield Views. (Closed)

Created:
9 years, 11 months ago by oshima

Modified:
9 years, 6 months ago

Reviewers:
Ben Goodger (Google)

CC:
chromium-reviews, Paweł Hajdan Jr.

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

Visibility:
Public.

Description

Add TextRange and GetSelectedRange, SelectRange and GetCursorPosition, which are necessary to implement ominibox. I'm adding them as Views only and left win/gtk blank because we don't use win/gtk impl for omnibox. I also didn't add TextRange variable to Textfield as its state is kept in Model. Let me know if you want to add it in Textfield class as well. BUG=none TEST=added new tests RangeTest, SelectRangeTest Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=71811

Patch Set 1 #

Patch Set 2 : " #

Patch Set 3 : " #

Patch Set 4 : " #

Patch Set 5 : " #

Patch Set 6 : " #

Total comments: 6

Patch Set 7 : " #

Created: 9 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+325 lines, -26 lines)			Patch
M	views/controls/textfield/native_textfield_gtk.h	View	1 2 3 4	2 chunks	+4 lines, -1 line	0 comments	Download
M	views/controls/textfield/native_textfield_gtk.cc	View	1 2 3 4	2 chunks	+14 lines, -1 line	0 comments	Download
M	views/controls/textfield/native_textfield_views.h	View		1 chunk	+3 lines, -0 lines	0 comments	Download
M	views/controls/textfield/native_textfield_views.cc	View		1 chunk	+14 lines, -2 lines	0 comments	Download
M	views/controls/textfield/native_textfield_win.h	View	1 2 3 4	3 chunks	+5 lines, -1 line	0 comments	Download
M	views/controls/textfield/native_textfield_win.cc	View	1 2 3 4	2 chunks	+14 lines, -1 line	0 comments	Download
M	views/controls/textfield/native_textfield_wrapper.h	View	1 2 3 4	3 chunks	+11 lines, -1 line	0 comments	Download
M	views/controls/textfield/textfield.h	View	1 2 3 4 5 6	3 chunks	+62 lines, -2 lines	0 comments	Download
M	views/controls/textfield/textfield.cc	View	1 2 3 4 5	3 chunks	+39 lines, -1 line	0 comments	Download
M	views/controls/textfield/textfield_views_model.h	View	1 2 3 4 5 6	3 chunks	+10 lines, -0 lines	0 comments	Download
M	views/controls/textfield/textfield_views_model.cc	View	1 2 3 4 5 6	4 chunks	+18 lines, -1 line	0 comments	Download
M	views/controls/textfield/textfield_views_model_unittest.cc	View	1 2 3 4	11 chunks	+131 lines, -15 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Ben Goodger (Google)

LGTM. http://codereview.chromium.org/6318004/diff/12001/views/controls/textfield/textfield.h File views/controls/textfield/textfield.h (right): http://codereview.chromium.org/6318004/diff/12001/views/controls/textfield/textfield.h#newcode236 views/controls/textfield/textfield.h:236: // Gets the selected range. This is views ...

9 years, 11 months ago (2011-01-19 16:40:16 UTC) #2

oshima

9 years, 11 months ago (2011-01-19 18:22:52 UTC) #3

thank you for prompt review!

http://codereview.chromium.org/6318004/diff/12001/views/controls/textfield/te...
File views/controls/textfield/textfield.h (right):

http://codereview.chromium.org/6318004/diff/12001/views/controls/textfield/te...
views/controls/textfield/textfield.h:236: // Gets the selected range. This is
views implementation only and
On 2011/01/19 16:40:16, Ben Goodger wrote:
> here, and everywhere else: views-implementation

Done.

http://codereview.chromium.org/6318004/diff/12001/views/controls/textfield/te...
views/controls/textfield/textfield.h:244: // Returns the currnet cursor
position. This is views implementation
On 2011/01/19 16:40:16, Ben Goodger wrote:
> current

Done.

http://codereview.chromium.org/6318004/diff/12001/views/controls/textfield/te...
File views/controls/textfield/textfield_views_model.h (right):

http://codereview.chromium.org/6318004/diff/12001/views/controls/textfield/te...
views/controls/textfield/textfield_views_model.h:160: size_t SafePosition(size_t
position) const;
On 2011/01/19 16:40:16, Ben Goodger wrote:
> GetSafePosition for consistency?

Done.

Expand Messages | Collapse Messages