Closed Bug 1471951 Opened Last year Closed Last year

Support Talkback editing actions (select/copy/paste)

Categories

(Core :: Disability Access APIs, enhancement, P1)

All
Android
enhancement

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox-esr52 --- wontfix
firefox-esr60 --- wontfix
firefox61 --- wontfix
firefox62 --- wontfix
firefox63 --- fixed

People

(Reporter: eeejay, Assigned: eeejay)

References

Details

(Whiteboard: [geckoview:klar])

Attachments

(2 files, 2 obsolete files)

This is done in the local context menu, under the "Editing" sub menu. It has the following options:

* Move cursor to beginning
* Move cursor to end
* Paste
* Select all
* Start/stop selection mode

The last option makes that when you move the caret by text granularity it grows or shrinks the selection range.
Depends on: 1472274
Priority: -- → P1
Attachment #8991168 - Flags: review?(yzenevich)
Attachment #8991168 - Flags: review?(nchen)
This will benefit from the line navigation changes in bug 1474688 as well. So that is great!
Attachment #8991169 - Flags: review?(yzenevich)
Attachment #8991169 - Flags: review?(nchen)
Attachment #8991168 - Flags: review?(nchen) → review+
Attachment #8991169 - Flags: review?(nchen) → review+
Comment on attachment 8991168 [details] [diff] [review]
Support set selection and clipboard actions (1/2). r?yzen r?jchen

Review of attachment 8991168 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, a nit regarding prefering const to let anywhere we do not reassign variables in JS.

::: accessible/jsat/EventManager.jsm
@@ +213,5 @@
> +          if (acc.selectionCount) {
> +            let [startSel, endSel] = Utils.getTextSelection(acc);
> +            let fromIndex = startSel == caretOffset ? endSel : startSel;
> +            this.present(Presentation.textSelectionChanged(
> +              acc.getText(0, -1), fromIndex, caretOffset, 0, 0,

looks like here the only difference is fromIndex vs caretOffset in the else statement. can we just have one if () no else where we set fromIndex if acc.selectionCount is truthy otherwise defaulting to the value of caretOffset?
Attachment #8991168 - Flags: review?(yzenevich) → review+
Comment on attachment 8991169 [details] [diff] [review]
Support expand selection with caret (2/2). r?yzen r?jchen

Review of attachment 8991169 [details] [diff] [review]:
-----------------------------------------------------------------

thanks! same suggestions about const over let where possible.
Attachment #8991169 - Flags: review?(yzenevich) → review+
Pushed by eisaacson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a682e5c249b8
Support set selection and clipboard actions (1/2). r=yzen r=jchen
https://hg.mozilla.org/integration/mozilla-inbound/rev/78356edc6a51
Support expand selection with caret (2/2). r=yzen r=jchen
That's silly. I'll fix and repush. Sorry!
Flags: needinfo?(eitan)
Attachment #8991168 - Attachment is obsolete: true
Attachment #8991169 - Attachment is obsolete: true
Keywords: checkin-needed
Pushed by eisaacson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/590ba4c10edd
Support set selection and clipboard actions (1/2). r=yzen r=jchen
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb7687c97e1f
Support expand selection with caret (2/2). r=yzen r=jchen
Keywords: checkin-needed
[geckoview:klar] so this bug will be triaged as a potential Focus+GV blocker.
Pushed by eisaacson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1839e4b613bd
Support set selection and clipboard actions (1/2). r=yzen r=jchen
https://hg.mozilla.org/integration/mozilla-inbound/rev/d75a549c4e93
Support expand selection with caret (2/2). r=yzen r=jchen
Flags: needinfo?(eitan)
https://hg.mozilla.org/mozilla-central/rev/1839e4b613bd
https://hg.mozilla.org/mozilla-central/rev/d75a549c4e93
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
status-firefox62=wontfix because Eitan recommends that we do not uplift this fix to GV 62 beta.
You need to log in before you can comment on or make changes to this bug.