Issue metadata
Sign in to add a comment
|
Don't allow start/begin selection handle to change places |
||||||||||||||||||||
Issue descriptionVersion: all versions I tried: 49.0.2623.91, 50.0.2661.49, 51.0.2687 OS: Android What steps will reproduce the problem? (1) Select text (2) Drag the "end" selection handle to be before the "start" selection handle (or the other way around) (3) Observe the orientation of the handle you are dragging. What is the expected output? The orientation of the handle should change once it changes its relative position (i.e. once the end handle becomes the start handle) What do you see instead? The orientation of the handle doesn't change until you stop dragging it. Note that the orientation of the other handle does update properly
,
Mar 25 2016
I can repro, and indeed doesn't repro in M45 at least. I guess a recent patch in the area such as my https://codereview.chromium.org/1531473004 regressed this.
,
Mar 26 2016
I believe this is working as intended: see issue 544095 (not sure why it is not marked as fixed).
,
Mar 26 2016
I definitely observed the strange behavior on a recent build. But notice that I only checked Android. Issue 544095 seems to perhaps only fix it for Aura?
,
Mar 26 2016
Re #4: Issue 544095 is requesting what you call "the strange behavior". However, it is only "fixing" it for Android because, I believe, it is currently mostly useful on Android where handles can be flipped/mirrored based on the location inside the viewport. Does that make sense?
,
Mar 26 2016
Re #5: I may not be understanding correctly. Would you mind helping me find where I'm not following correctly? I believe Issue 544095 asks to not swap handle orientation, since the user felt it wasn't clear that you were dragging the same handle. And I believe the decision was to swap the handle orientations to maintain the start/end look. The problem I am observing is we have 2 end orientations and 0 start orientations. It is only like that while you are dragging the end handle past the start. Once you release your finger it becomes the start orientation. (It may also happen going the other direction. I didn't test that.)
,
Mar 27 2016
Re #6: Yes, issue 544095 asks not to change orientation of the handle that is being dragged until it is released and that's what r366031 has implemented. It makes sense to me, as changing handle orientation creates a gap between the finger and the handle (especially with adaptive handles enabled). There was also a plan to have floating handles that follow the finger instead of selection start/end in which case this change makes even more sense. I can't find the decision to maintain the start/end look (as you mentioned) in the bug or the CL comments. Is this a new decision?
,
Mar 27 2016
Re #7: OOOhhhh okay. I am not aware of any special decision. So the currently-dragged handle intentionally remains unflipped so it stays where it was relative to your finger. I understand. In that case I agree, this is working as intended.
,
Mar 28 2016
Hmm, OK, seems like a UX difference of opinion. As for my opinion on that, I tend to agree with mfomitchev@ that changing orientation ASAP was better than the current behavior. The reason is that the orientation switch on finger release is surprising and distracting. The fact that mfomitchev@ was bothered enough to file a bug indicates it's *quite* distracting. So I'd be inclined to revert https://codereview.chromium.org/1404163004. mohsen@, what do you think? (I checked what Android TextView does in this case, and TextView does not even allow swapping handle order -- it blocks moving the handle when exactly one character is selected -- so that doesn't help decide this. I'm not super inclined to go out of our way to match Android behavior on this because Chromium's behavior seems more useful.)
,
Mar 29 2016
I feel current behavior is *a bit* better for the reasons I mentioned in c#7; however, I really don't have a strong opinion about it.
,
Mar 29 2016
Swapping the handle order: When we were working on Android M text selection for Chrome, this did come up. My position was exactly the same as aelias's - I thought the fact that Chrome supported swapping the handle order made Chrome's implementation superior. However Android UX had a different take on the matter. Here's a comment from ruqianz@ from an old thread: "On Android platform we intentionally set the rule to handles that one handle never surpasses the other one. We decided that the crossover behavior was disorienting and we wanted to make sure to avoid it." Apparently they did some user studies on this, and this was the outcome. I am still not super convinced, but it might be worth digging into this a bit more. Changing the handle orientation (if we still allow swapping the handle order): I understand the problem raised in issue 544095 , but yeah, the current behavior just looks [more] broken to me. I'd be in favor of reverting the change. If there was some support from UX/UXR for the current behavior, I might change my opinion.
,
Mar 29 2016
OK, thanks for the update. Since the Android UX decision was conscious and backed by actual user studies, then I think the correct resolution in this area is to match TextView behavior after all. I can see the disorientation argument -- particularly when a user might be trying to select just a few words and ends up overshooting.
,
Mar 29 2016
SGTM. Notably the question of whether the orientation should change during the drag still stands even if we don't allow the handle order to change (e.g see video in issue 544095 ). However in this case I think not changing the orientation becomes more defensible. On a related note, we should consider actually enabling the adaptive handle orientation by default (i.e. the behavior shown in videos in issue 544095 where handles flip if needed to stay in the content area) . This is controlled from https://code.google.com/p/chromium/codesearch#chromium/src/ui/touch_selection/touch_selection_controller.h&sq=package:chromium&type=cs&l=62&rcl=1459265830. Now, to make it so once handle can never surpass the other, one needs to make a change in third_party/WebKit/Source/core/editing/GranularityStrategy.cpp, specificallty DirectionGranularityStrategy::updateExtent(). Incidentally, this should simplify that code quite a bit - the case where extentBaseOrderSwitched is true would become a lot simpler - we'd just set the selection to be one character long.
,
Mar 29 2016
OK, assigning this back to myself to track.
,
Mar 30 2016
Thanks aelias@, mfomitchev@, mohsen@ Just wanted to add some observations while making the change (crrev.com/1404163004): 1. As mentioned in the bug, the disconnect between the handles and the touch position of the finger is more prominent with Adaptive Touch Handles enabled. When disabled, the change in position is minimal which would make the user feel that he is still *dragging* the handles even when there is an orientation switch. Regarding comment from ruqianz@: The user studies were based on the behavior without Adaptive Handle Orientation? If that is not the case, then I would expect that the crossover behavior was disorienting. This would be good to dig into this a bit more. Also, IMHO, when Floating handles is enabled, the gap between the touch position and the handle location would be more distracting than the switch that happens on touch_up. 2. From code flow perspective on TouchSelectionController side, we only set the is_dragging flag on touch down and then continue to change the position of handles based on the updates from UpdateSelection(). This means that after swap happens (while dragging the end handle), even though the user is *dragging* the start handle (after swap), the flag would be set only for the end_handle (as that was the one that was touched initially). This doesn't cause any issues, but logically makes sense to set the flag for the appropriate handle.
,
Aug 14 2017
Putting on Pedro's backlog. Renaming to match conclusion in #12 that we should match TextView. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by mfomitchev@chromium.org
, Mar 25 2016