New issue
Advanced search Search tips

Issue 641420 link

Starred by 0 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug
Team-Accessibility



Sign in to add a comment

Swiping doesn't move cursor in contenteditable

Project Member Reported by paulmiller@chromium.org, Aug 26 2016

Issue description

from internal bug 31020872

When editing in a native EditText or an <input type="text"> in Chrome, swiping left and right moves the cursor. In a <div contenteditable="true"> in Chrome, swiping moves the announcement but doesn't move the cursor.
 
I repro in 52.0.2743.99 and 40.0.2214.89. This probably never worked.
Cc: dmazz...@chromium.org
Owner: paulmiller@chromium.org
Status: Assigned (was: Untriaged)
It looks like content-editables and text inputs are a bit different.

Content-editables have an announcement position which is separate from the cursor. Text inputs don't; if I prevent the cursor from moving, then neither does the announcement.

Content-editables don't necessarily have AX_ROLE_TEXT_FIELD (which is conceptually weird, but necessary if AXObjects can't have multiple roles), therefore IsEditableText() on content-editables returns false, therefore BrowserAccessibilityManager.finishGranularityMove() doesn't update the cursor the way it does for text inputs.
It seems that the volume rockers don't move the cursor in content-editables either. Not sure if that's part of this bug or not.

The pseudo stack trace that moves a text input cursor, when swiping or using the volume buttons, is:

HTMLTextFormControlElement::setSelectionRange
AXLayoutObject::setSelection
WebAXObject::setSelection
RenderAccessibilityImpl::OnSetSelection
(IPC)
RenderFrameHostImpl::AccessibilitySetSelection
BrowserAccessibilityManager::SetTextSelection
BrowserAccessibilityManager::SetSelection
(JNI)
BrowserAccessibilityManagerAndroid.finishGranularityMove
(JNI)
BrowserAccessibilityManagerAndroid::Next/PreviousAtGranularity
(JNI)
BrowserAccessibilityManager.next/previousAtGranularity
BrowserAccessibilityManager.performAction

For content-editables, the following conditions prevent this path from executing on swipes:

HTMLTextFormControlElement::setSelectionRange - isTextFormControl returns false
AXLayoutObject::setSelection - isTextControl returns false
BrowserAccessibilityManager.finishGranularityMove - nativeIsEditableText returns false

(On volume rocker presses, performAction doesn't happen at all.)
When dragging the cursor without TalkBack, in both text inputs and content-editables, the pseudo stack is:

FrameSelection::setSelectionAlgorithm<blink::EditingAlgorithm<blink::NodeTraversal> >
FrameSelection::setSelection
FrameSelection::moveTo
WebLocalFrameImpl::moveCaretSelection
RenderViewImpl::OnMoveCaret
(IPC)
RenderWidgetHostImpl::MoveCaret
RenderWidgetHostViewAndroid::MoveCaret
TouchSelectionController::OnDragUpdate
TouchHandle::WillHandleTouchEvent
TouchSelectionController::WillHandleTouchEvent
RenderWidgetHostViewAndroid::OnTouchEvent
ContentViewCoreImpl::OnTouchEvent
(JNI)
ContentViewCore.onTouchEventImpl

FrameSelection::setSelection is also called by HTMLTextFormControlElement::setSelectionRange, so that's where the AX and non-AX paths join up.

If I hack around the IsEditableText check in BrowserAccessibilityManager.finishGranularityMove, then AXLayoutObject::setSelection falls through and calls FrameSelection::setSelection, which does actually move the cursor. But it looks like the toVisiblePosition calculation is mangling the cursor offsets. Any non-0 offset gets warped to the end of the text. So while the announcement is jumping back and forth by the granularity setting, the cursor warps between the start and end.
Cc: paulmiller@chromium.org
Owner: nek...@chromium.org
From Dominic: "The problem isn't the offsets exactly, it's with the object. finishGranularityMove needs to go into the children of the contenteditable and find the text node to place the offset. It's trying to set offsets on the <div> - but the div just consists of an element with a single child. What we actually want is to set the selection on the text inside the div."

Passing to Nektarios. To summarize, we should:

1) Fix finishGranularityMove (or nativeIsEditableText) so that nativeSetSelection is called for content-editables.
2) Convert from offset to deep offset in finishGranularityMove.
3) Add test coverage. The existing tests:
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/accessibility/contenteditable-selection.html
don't seem to exercise finishGranularityMove.
Labels: NewComponent-Accessibility NewComponent-Accessibility-Compatibility
Components: UI>Accessibility>Compatibility
Components: -UI>Accessibility
Labels: -newcomponent-accessibility-compatibility -newcomponent-accessibility
Labels: triage-android-remaining
Owner: dmazz...@chromium.org
Status: Available (was: Assigned)
Status: Assigned (was: Available)
Labels: -Pri-2 Pri-3

Sign in to add a comment