Implement double/triple click functionality in views textfield.

Implement double/triple click functionality in views textfield. Also changed views::Event::time_stamp_ to a platform independent time stamp.

@beng: please review views::Event changes (event.cc,event.h)
@oshima: please review the rest

BUG=none
TEST=new tests added.

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

[oshima, 2011-01-18 19:52:53]

http://codereview.chromium.org/6267002/diff/3001/views/controls/textfield/nat...
File views/controls/textfield/native_textfield_views.cc (right):

http://codereview.chromium.org/6267002/diff/3001/views/controls/textfield/nat...
views/controls/textfield/native_textfield_views.cc:770: return false;
1) mouse move (with some threshold) should cancel the double/triple clicks.
2) the 3rd click should be same as single click (cursor selection)

It'd be clearer if this returns the type of action to take, and the caller
invokes the methods on model, something like

switch (GetClickAction(event)) {
  case CursorSelect:
  case SelectWord:
  case SelectAll:
}

http://codereview.chromium.org/6267002/diff/3001/views/controls/textfield/tex...
File views/controls/textfield/textfield_views_model.cc (right):

http://codereview.chromium.org/6267002/diff/3001/views/controls/textfield/tex...
views/controls/textfield/textfield_views_model.cc:227: void
TextfieldViewsModel::SelectWord() {
Will this generate same word boundary that break iterator generates?

http://codereview.chromium.org/6267002/diff/3001/views/event.h
File views/event.h (right):

http://codereview.chromium.org/6267002/diff/3001/views/event.h#newcode85
views/event.h:85: base::Time GetTimeStamp() const {
const base::Time&

[Ben Goodger (Google), 2011-01-18 20:05:22]

Event changes LGTM with the point oshima noted.

[varunjain, 2011-01-25 04:06:40]

http://codereview.chromium.org/6267002/diff/3001/views/controls/textfield/nat...
File views/controls/textfield/native_textfield_views.cc (right):

http://codereview.chromium.org/6267002/diff/3001/views/controls/textfield/nat...
views/controls/textfield/native_textfield_views.cc:770: return false;
On 2011/01/18 19:52:54, oshima wrote:
> 1) mouse move (with some threshold) should cancel the double/triple clicks.
> 2) the 3rd click should be same as single click (cursor selection)
> 
> It'd be clearer if this returns the type of action to take, and the caller
> invokes the methods on model, something like
> 
> switch (GetClickAction(event)) {
>   case CursorSelect:
>   case SelectWord:
>   case SelectAll:
> }
> 
> 

Done.

http://codereview.chromium.org/6267002/diff/3001/views/controls/textfield/tex...
File views/controls/textfield/textfield_views_model.cc (right):

http://codereview.chromium.org/6267002/diff/3001/views/controls/textfield/tex...
views/controls/textfield/textfield_views_model.cc:227: void
TextfieldViewsModel::SelectWord() {
On 2011/01/18 19:52:54, oshima wrote:
> Will this generate same word boundary that break iterator generates?

no... particularly, the handling of non A-Z chars are very different.

http://codereview.chromium.org/6267002/diff/3001/views/event.h
File views/event.h (right):

http://codereview.chromium.org/6267002/diff/3001/views/event.h#newcode85
views/event.h:85: base::Time GetTimeStamp() const {
On 2011/01/18 19:52:54, oshima wrote:
> const base::Time&

Done.

[oshima, 2011-01-25 19:31:58]

http://codereview.chromium.org/6267002/diff/19001/views/controls/textfield/na...
File views/controls/textfield/native_textfield_views.cc (right):

http://codereview.chromium.org/6267002/diff/19001/views/controls/textfield/na...
views/controls/textfield/native_textfield_views.cc:88: if
(model_->MoveCursorTo(pos, false)) {
It's probably better to move this logic to GetClickAction and
DetectAndHandleDoubleAndTripleClick, rename DetectAndHandleDbouleAndTripleClick
to HandleMousePressed(e);
then do 
if (HandleMousePressed(e))
    SchedulePaint();
return true;

http://codereview.chromium.org/6267002/diff/19001/views/controls/textfield/na...
views/controls/textfield/native_textfield_views.cc:819: if
(tracking_double_click_) {
can't we make this simple state machine rather than using two variable?

http://codereview.chromium.org/6267002/diff/19001/views/controls/textfield/na...
File views/controls/textfield/native_textfield_views_unittest.cc (right):

http://codereview.chromium.org/6267002/diff/19001/views/controls/textfield/na...
views/controls/textfield/native_textfield_views_unittest.cc:460:
EXPECT_FALSE(IsTrackingTripleClick());
We should test one more time to see if it's going to be treated as single click.

[varunjain, 2011-01-25 20:10:40]

http://codereview.chromium.org/6267002/diff/19001/views/controls/textfield/na...
File views/controls/textfield/native_textfield_views.cc (right):

http://codereview.chromium.org/6267002/diff/19001/views/controls/textfield/na...
views/controls/textfield/native_textfield_views.cc:88: if
(model_->MoveCursorTo(pos, false)) {
On 2011/01/25 19:31:59, oshima wrote:
> It's probably better to move this logic to GetClickAction and
> DetectAndHandleDoubleAndTripleClick, rename
DetectAndHandleDbouleAndTripleClick
> to HandleMousePressed(e);
> then do 
> if (HandleMousePressed(e))
>     SchedulePaint();
> return true;

Done.

http://codereview.chromium.org/6267002/diff/19001/views/controls/textfield/na...
views/controls/textfield/native_textfield_views.cc:819: if
(tracking_double_click_) {
On 2011/01/25 19:31:59, oshima wrote:
> can't we make this simple state machine rather than using two variable?

Done.

http://codereview.chromium.org/6267002/diff/19001/views/controls/textfield/na...
File views/controls/textfield/native_textfield_views_unittest.cc (right):

http://codereview.chromium.org/6267002/diff/19001/views/controls/textfield/na...
views/controls/textfield/native_textfield_views_unittest.cc:460:
EXPECT_FALSE(IsTrackingTripleClick());
On 2011/01/25 19:31:59, oshima wrote:
> We should test one more time to see if it's going to be treated as single
click.

Done.

[varunjain, 2011-01-25 22:53:41]

@oshima: PTAL

[oshima, 2011-01-25 23:10:21]

LGTM

Thanks!