Add and specify Views::Textfield::SelectAll |reversed| flag, etc.

Add and specify Views::Textfield::SelectAll |reversed| flag, etc.

SelectAll should be consistent and explicit with range reversal.
This dictates if leading or trailing text is shown when textfields overflow.
Currently, NativeTextfield[Win|Views]::SelectAll behavior is implicit and differs.
 (Windows native reverses the selection, while ChromeOS/Views doesn't)

Revise SelectAll in RenderText, NativeTextfieldWin, and OmniboxViewViews.
Add the |reversed| parameter and plumbing to related interfaces/functions.
Add/update RenderTextTest.SelectAll and TextfieldViewsModelTest.Selection.

Specify explicit reversal behavior in all the following cases:

1) Use reversed selection (changes ChromeOS/Views behavior) in:
a) BookmarkBubbleView::ShowBubble (focus on bookmark title when shown).
b) BookmarkEditorView::Accept (focus on invalid bookmark URL on "Save").
c) BookmarkEditorView::Show (focus on bookmark title when shown).
d) FindBarView::UpdateForResult (find bar matches are found/iterated).
e) FindBarView::SetFocusAndSelection (find bar shown, etc.).
f) FindBarView::SearchTextfieldView::RequestFocus (click find bar parts, etc.).
g) EditSearchEngineDialog::Show (focus on search engine title when shown).
h) LoginView::OnAutofillDataAvailable (HTTP/FTP auth window shown).
i) MessageBoxView::ViewHierarchyChanged (JS dialog with text input shown).

2) Use forward selection (changes Windows native behavior) in:
a) NativeTextfieldWin::ExecuteCommand (textfield context menu "Select All").
 (note: the Omnibox context menu "Select All" already uses forward selection)
b) Textfield::AboutToRequestFocusFromTabTraversal (focus via tab-traversal).
 (note1: THIS IS CONTENTIOUS! Though OmniBoxViewWin is unaffected)
 (note2: OmniboxViewViews should be fixed later as per crbug.com/134701#c9)
c) TreeView::StartEditing (editing tree view nodes ex/ collected cookies).
d) NativeTextfieldViewsTest.* and ViewTest.* (changes inconsequential to tests)

3) Formally specify existing implicit behavior (no behavioral change):
a) NativeTextfieldWin::OnAfterPossibleChange (temporary selection is reversed).
b) NativeTextfieldViews::OnGestureEvent (double tap is forwards).
c) NativeTextfieldViews::ExecuteCommand (context menu "Select All" is forwards).
d) NativeTextfieldViews::HandleKeyEvent (CTRL-A is forwards).
e) NativeTextfieldViews::HandleMousePressEvent (triple-click is forwards).
f) TextfieldViewsModel::SetText (temporary selection is forwards).
g) TextfieldViewsModelTest.* is mostly forwards, |Selection| tests reversed.

TBR=ben@chromium.org
BUG=134762
TEST=New RenderTextTest.SelectAll, updated TextfieldViewsModelTest.Selection, other unit tests, manual.

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

[msw, 2012-07-12 06:47:44]

Hey Peter, please take a look and consider these behavioral changes; thanks!

[msw, 2012-07-12 11:13:15]

Amidst actual use, reversed selections seem generally better because they leave
(or make) the leading text of long strings visible on select-all. This tends to
be more informative and less confusing/jarring. Otherwise, forward selections
more often pan from the leading portion of the text to show the trailing text
(especially on select-all when the cursor was at position 0 with no selection).

I think most of the cases in group (2) should actually keep the current native
Windows behavior of reversed selections (and change ChromeOS/Views to match),
especially on focus via tab-traversal, case (e).

I would even suggest considering reversed selections for all select-all uses in
Chrome like triple-click and CTRL-A. Peter, can you elaborate on why you think
this is wrong?

It may also be worthwhile to explore a solution (for some instances?) where
select-all automatically determines its direction based on the minimal
disruption to the existing selection range and/or cursor position.

[Peter Kasting, 2012-07-12 18:19:28]

In most cases I think the "panning" effect is OK; for example, if I have my
cursor at the beginning of the omnibox and hit ctrl-A I expect the results to
pan and am somewhat surprised if they don't, because long experience has trained
me that that's how "select all" works on Windows.  (Furthermore, if I then try
to hold shift-left to remove some characters at the end of the selection, I'll
be kind of annoyed if it doesn't work.)

I would perhaps change (2)(a), (b), (c) to use reversed selections since it
sounds like those are cases similar to (1)(a) where we show some bunch of text
as selected-by-default.

[msw, 2012-07-12 19:36:27]

Okay, I flipped the 3 cases mentioned. I think changing Windows tab-traversal
behavior (2b) is the most disruptive change here, but seems okay. Please take
another look; thanks!

[Peter Kasting, 2012-07-13 00:52:04]

I think the "Select reversed so we show the beginning in case of overlow"
comments are kind of restating what the code already does.  It might be nice to
instead change some of these to give motivation for why we want this, like
"Reverse the selection so we show the beginning of the page title when the
bookmark bubble appears, since this will be more meaningful to the user, while
showing the end may just look confusing."  Or just eliminate them.

Rubber-stamp LGTM otherwise.

[msw, 2012-07-13 01:05:01]

I just decided to remove the redundant comments.
I'm going to TBR=ben@chromium.org for ui/views/OWNERS.

[commit-bot: I haz the power, 2012-07-13 01:05:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/10693160/5007

[commit-bot: I haz the power, 2012-07-13 02:09:17]

Try job failure for 10693160-5007 (retry) on mac_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-07-13 02:47:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/10693160/5007

[commit-bot: I haz the power, 2012-07-13 04:44:57]

Change committed as 146520