MacViews: Select all text on right clicking an unfocused text view.

MacViews: Select all text on right clicking an unfocused text view.

When an unfocused Cocoa text view is right clicked, all its text is selected.
This CL implements this behavior on MacViews for views::Textfield and
views::Label. A new variable kSelectAllOnRightClickWhenUnfocused is introduced
to the PlatformStyle class to account for the platform specific differences.
SelectionController::OnMousePressed is also modified to take the initial focus
state of its client view as an argument. A test is also added to
textfield_unittest.cc.

BUG=676296
Review-Url: https://codereview.chromium.org/2650963002
Cr-Commit-Position: refs/heads/master@{#446588}
Committed: https://chromium.googlesource.com/chromium/src/+/81dc16e8871f33d24d0e12b0020491ee6ca18399

[karandeepb, 2017-01-24 03:59:48]

Description was changed from

==========
MacViews: Select all text on right clicking an unfocused text view.
==========

to

==========
MacViews: Select all text on right clicking an unfocused text view.

When an unfocused Cocoa text view is right clicked, all its text is selected.
This CL implements this behavior on MacViews for views::Textfield and
views::Label. A new variable kSelectAllOnRightClickWhenUnfocused is introduced
to the PlatformStyle class to account for the platform specific differences.
SelectionController::OnMousePressed is also modified to take the initial focus
state of its client view as an argument. A test is also added to
textfield_unittest.cc.

BUG=676296
==========

[karandeepb, 2017-01-24 03:59:56]

The CQ bit was checked by karandeepb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-24 04:00:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[karandeepb, 2017-01-24 04:13:49]

The CQ bit was checked by karandeepb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-24 04:14:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[karandeepb, 2017-01-24 04:52:34]

karandeepb@chromium.org changed reviewers:
+ tapted@chromium.org

[karandeepb, 2017-01-24 04:52:35]

PTAL Trent.

[commit-bot: I haz the power, 2017-01-24 05:14:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-24 05:14:14]

Dry run: This issue passed the CQ dry run.

[karandeepb, 2017-01-25 06:45:04]

karandeepb@chromium.org changed reviewers:
+ msw@chromium.org
- tapted@chromium.org

[karandeepb, 2017-01-25 06:45:04]

Seems Trent missed this. msw@ can you review this, since tomorrow it's a holiday
here.

[tapted, 2017-01-25 09:11:15]

tapted@chromium.org changed reviewers:
+ tapted@chromium.org

[tapted, 2017-01-25 09:11:16]

Sorry! lgtm - looks pretty good! But msw is a more specific OWNER (in
ui/views/controls/textfield) and knows this stuff better than me, so he should
take a look :)

https://codereview.chromium.org/2650963002/diff/20001/ui/views/controls/label.cc
File ui/views/controls/label.cc (right):

https://codereview.chromium.org/2650963002/diff/20001/ui/views/controls/label...
ui/views/controls/label.cc:512: const bool has_focus = HasFocus();
optional: maybe had_focus?

https://codereview.chromium.org/2650963002/diff/20001/ui/views/controls/textf...
File ui/views/controls/textfield/textfield.cc (right):

https://codereview.chromium.org/2650963002/diff/20001/ui/views/controls/textf...
ui/views/controls/textfield/textfield.cc:578: const bool has_focus = HasFocus();
had_focus

[karandeepb, 2017-01-25 10:40:08]

Patchset #3 (id:40001) has been deleted

[karandeepb, 2017-01-25 10:43:56]

Will wait for msw@ to take a look.

https://codereview.chromium.org/2650963002/diff/20001/ui/views/controls/label.cc
File ui/views/controls/label.cc (right):

https://codereview.chromium.org/2650963002/diff/20001/ui/views/controls/label...
ui/views/controls/label.cc:512: const bool has_focus = HasFocus();
On 2017/01/25 09:11:16, tapted wrote:
> optional: maybe had_focus?

Done.

https://codereview.chromium.org/2650963002/diff/20001/ui/views/controls/textf...
File ui/views/controls/textfield/textfield.cc (right):

https://codereview.chromium.org/2650963002/diff/20001/ui/views/controls/textf...
ui/views/controls/textfield/textfield.cc:578: const bool has_focus = HasFocus();
On 2017/01/25 09:11:16, tapted wrote:
> had_focus

Done.

[msw, 2017-01-25 19:23:54]

lgtm with a minor q; thanks!

https://codereview.chromium.org/2650963002/diff/80001/ui/views/controls/textf...
File ui/views/controls/textfield/textfield.cc (right):

https://codereview.chromium.org/2650963002/diff/80001/ui/views/controls/textf...
ui/views/controls/textfield/textfield.cc:582: RequestFocus();
q: should this RequestFocus call also be in an if (!had_focus) conditional?

[karandeepb, 2017-01-27 02:29:37]

https://codereview.chromium.org/2650963002/diff/80001/ui/views/controls/textf...
File ui/views/controls/textfield/textfield.cc (right):

https://codereview.chromium.org/2650963002/diff/80001/ui/views/controls/textf...
ui/views/controls/textfield/textfield.cc:582: RequestFocus();
On 2017/01/25 19:23:54, msw wrote:
> q: should this RequestFocus call also be in an if (!had_focus) conditional?

Done. Though this shouldn't matter anyway, RequestFocus on the currently focused
view will be a no-op.

[karandeepb, 2017-01-27 02:29:42]

The CQ bit was checked by karandeepb@chromium.org

[karandeepb, 2017-01-27 02:29:42]

The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org,
msw@chromium.org
Link to the patchset: https://codereview.chromium.org/2650963002/#ps100001
(title: "Nit")

[commit-bot: I haz the power, 2017-01-27 02:30:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-27 04:12:49]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1485484182217480,
"parent_rev": "b875cd72ebc343cdd479e8cdf1fc79dcb0f833db", "commit_rev":
"81dc16e8871f33d24d0e12b0020491ee6ca18399"}

[commit-bot: I haz the power, 2017-01-27 04:13:47]

Description was changed from

==========
MacViews: Select all text on right clicking an unfocused text view.

When an unfocused Cocoa text view is right clicked, all its text is selected.
This CL implements this behavior on MacViews for views::Textfield and
views::Label. A new variable kSelectAllOnRightClickWhenUnfocused is introduced
to the PlatformStyle class to account for the platform specific differences.
SelectionController::OnMousePressed is also modified to take the initial focus
state of its client view as an argument. A test is also added to
textfield_unittest.cc.

BUG=676296
==========

to

==========
MacViews: Select all text on right clicking an unfocused text view.

When an unfocused Cocoa text view is right clicked, all its text is selected.
This CL implements this behavior on MacViews for views::Textfield and
views::Label. A new variable kSelectAllOnRightClickWhenUnfocused is introduced
to the PlatformStyle class to account for the platform specific differences.
SelectionController::OnMousePressed is also modified to take the initial focus
state of its client view as an argument. A test is also added to
textfield_unittest.cc.

BUG=676296

Review-Url: https://codereview.chromium.org/2650963002
Cr-Commit-Position: refs/heads/master@{#446588}
Committed:
https://chromium.googlesource.com/chromium/src/+/81dc16e8871f33d24d0e12b00204...
==========

[commit-bot: I haz the power, 2017-01-27 04:13:49]

Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/81dc16e8871f33d24d0e12b00204...