New issue
Advanced search Search tips

Issue 892216 link

Starred by 3 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Shift-up and shift-down should create partial text selections

Project Member Reported by jdonnelly@chromium.org, Oct 4

Issue description

Currently the shift modifier has no effect on the use of the up/down keys in the omnibox on most Views platforms. Shift-up and shift-down just move the suggestion selection up or down.

On Mac Chrome, however, as well as Edge, these shortcuts create a partial selection from the cursor to the beginning or end, respectively of the text in the omnibox.

We should enable this behavior on all Views platforms. See  https://crbug.com/863543 , for discussion.
 
Cc: sky@chromium.org
Components: Internals>Views
Ok, apparently Views doesn't support the shift-up and shift-down behavior. It looks easy to add however (by just duplicating what shift-home and shift-end do).

sky: any objections to me adding this behavior to the views text field?

The argument in favor of adding it is that:
a) shift-up and shift-down currently don't do anything so no functionality would be lost and
b) while not universal, this behavior is common on Windows. Text fields in dialogs in the control panel and Outlook do this. It also works in other Chrome surfaces (text fields in Blink and the search field at the top of various Web UI pages (chrome://settings, chrome://bookmarks, etc.)).
Cc: msw@chromium.org pkasting@chromium.org
IMO shift-up/down should behave as you outline. I'm not sure why we didn't implement this, may be an over sight. 
sgtm, please also consider updating selectable labels. The cookies dialog has single-line selectable labels that support Shift-Home/End, I think there are multi-line selectable labels somewhere (some dialog?)
Status: Started (was: Untriaged)
WIP CL for Textfield: https://crrev.com/c/1269110

msw: I'm not sure how to easily do what you're describing. I see that there are selectable labels and that label.cc has mouse event selection logic. But there isn't any existing notion of a cursor position that I can find that we would use to create a (cursor_position, start/end) selection. Can you elaborate on what you had in mind?
Your CL for Textfield makes sense (using TextEditCommand), I'd encourage following up separately for Label.

Label already handles Shift+Left/Right (not sure exactly where), so it shouldn't be too hard. views::Label has a SelectionController [1] that also uses the underlying gfx::RenderText [2], which stores the SelectionModel internally [3]. Perhaps the label key event handling would check if there's an existing selection and modify it for Shift+Up/Down, but I'd look closer to see how Labels already handle Shift+Left/Right.

[1] https://cs.chromium.org/chromium/src/ui/views/controls/label.h?rcl=7fbc699c08b42e562ecbdab2a4f632be7fffc685&l=374
[2] https://cs.chromium.org/chromium/src/ui/views/controls/label.h?rcl=7fbc699c08b42e562ecbdab2a4f632be7fffc685&l=340
[3] https://cs.chromium.org/chromium/src/ui/gfx/render_text.h?rcl=18a3e60dc9a297f4ad1aa6ab32587f6079f5e0b8&l=320
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ef3a78ab8098151ac5b2f8e34c2f0e718f614bee

commit ef3a78ab8098151ac5b2f8e34c2f0e718f614bee
Author: Justin Donnelly <jdonnelly@chromium.org>
Date: Thu Oct 11 14:25:54 2018

Enable shift-up and shift-down handling for text selection.

Bug: 892216
Change-Id: I728bde6f03e7896df093d1abb75027dba726fa47
Reviewed-on: https://chromium-review.googlesource.com/c/1269110
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598755}
[modify] https://crrev.com/ef3a78ab8098151ac5b2f8e34c2f0e718f614bee/ui/views/controls/textfield/textfield.cc
[modify] https://crrev.com/ef3a78ab8098151ac5b2f8e34c2f0e718f614bee/ui/views/controls/textfield/textfield_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0c03ac769cf38dd9108decf8776ad52149125968

commit 0c03ac769cf38dd9108decf8776ad52149125968
Author: Justin Donnelly <jdonnelly@chromium.org>
Date: Tue Oct 16 00:06:01 2018

[omnibox] Don't handle shift-up/down, let Textfield handle it instead.

Textfield creates partial selections from the point of the cursor on
shift-up/down.

Bug: 892216
Change-Id: I3196972dea1abeb5ffbf2d4381d27ca067e41bc6
Reviewed-on: https://chromium-review.googlesource.com/c/1276849
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599788}
[modify] https://crrev.com/0c03ac769cf38dd9108decf8776ad52149125968/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
[modify] https://crrev.com/0c03ac769cf38dd9108decf8776ad52149125968/chrome/browser/ui/views/omnibox/omnibox_view_views_unittest.cc

Sign in to add a comment