Accurate text selection using touch is currently almost impossible in Firefox. And once some text is selected, it's not possible to change the selection.

Now that we have touch events enabled on Windows (nightly only) we should consider turning on the touch selection carets there.

We could flip pref "layout.accessiblecaret.enabled" to true, and the blue carets on b2g should work on Windows, too.

I tried turning on the pref - the carets do show up and work, but there are bugs. For example when dragging around a caret to adjust the selection the page itself seems to scroll as well, and that causes the carets to jump/reposition unexpectedly as well as causes some slight flickering. We'll need to debug and fix these issues before we can turn it on.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #3) > For example when dragging around a caret to adjust the selection the > page itself seems to scroll as well, and that causes the carets to > jump/reposition unexpectedly as well as causes some slight flickering. This is the same as bug 1255555

The accessible caret seems to work better now that some of the bugs have been flushed out. We should turn it on in Nightly.

Oh, although it looks like turning on this pref makes text selection take priority over the context menu - so if you long-press on a link you get text selection handles instead of a context menu. :phlsa, any thoughts on what action the browser should take when long-pressing? On Fennec we do something like "if the thing you long-press is an image or link or other thing with specific context menu items, then show that, otherwise do text selection". On desktop even regular text still has a basic context menu, but presumably we want long-press to initiate text-selection in that case?

For Windows, I think the general heuristic should be to do whatever Edge is doing in terms of these micro-interactions. For links, Edge does both, select the link text AND show the context menu. When dismissing the context menu, the text stays selected. That seems sensible to me for Windows tablets.

So making it select the link and show the context menu was relatively easy to do. However just flipping the pref from false to true also has the effect of turning it on OS X and on other non-touch devices. This is noticeable if you use the mouse to select some text, you'll get the big blue caret handles which I assume is undesirable unless you have a touch-enabled device. I think what we want to do here is to hitch the accessible caret to whether or not touch events are enabled, the logic for which is pretty well encapsulated in TouchEvent::PrefEnabled.

On desktop, the context menu is shown when the user lifts their finger after a long press, but only if the eMouseLongTap event is not cancelled. So by not cancelling it, we allow both the text selection and the context menu. On Android, the context menu takes priority over text selection, so this has no effect (i.e. if the context menu is shown, then the AccessibleCaret code never even gets the eMouseLongTap event). Also on Android nothing else relies on the cancellation of the eMouseLongTap event, so this change is a no-op. Review commit: https://reviewboard.mozilla.org/r/68746/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/68746/

Review commit: https://reviewboard.mozilla.org/r/68748/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/68748/

Comment on attachment 8777119 [details] Bug 1195722 - On desktop, allow the context menu to pop up concurrently with text selection. https://reviewboard.mozilla.org/r/68746/#review65816 Thank you for analyzing the consumer of the long press event status. ::: layout/base/AccessibleCaretEventHub.cpp:326 (Diff revision 1) > > virtual nsEventStatus OnLongTap(AccessibleCaretEventHub* aContext, > const nsPoint& aPoint) override > { > - nsEventStatus rv = nsEventStatus_eIgnore; > - > + aContext->mManager->SelectWordOrShortcut(aPoint); > + return nsEventStatus_eIgnore; You'll need to fix the return value check for CreateLongTapEvent() in gtest AccessibleCaretEventHubTester::TestLongTapWithSelectWordSuccessful(). Nit: Might be worth adding a simple comment for the reason not consuming the event.

Comment on attachment 8777120 [details] Bug 1195722 - Add a new pref to enable the accessible carets if touch events are supported, and enable the pref on nightly. https://reviewboard.mozilla.org/r/68748/#review65818 ::: layout/base/nsPresShell.cpp:742 (Diff revision 1) > static bool sAccessibleCaretEnabled = false; > +static bool sAccessibleCaretOnTouch = false; > static bool sBeforeAfterKeyboardEventEnabled = false; > > /* static */ bool > -PresShell::AccessibleCaretEnabled() > +PresShell::AccessibleCaretEnabled(nsIDocShell* aDocShell) I'm a bit worry about getting a null aDocShell, but TouchEvent::PrefEnabled() seems check the validity of aDocShell before using. So I guess it's OK. ::: modules/libpref/init/all.js:5172 (Diff revision 1) > +// On Nightly, enable the accessible caret on platforms/devices > +// that we detect have touch support. Note that this pref is an > +// additional way to enable the accessible carets, rather than > +// overriding the layout.accessiblecaret.enabled pref. > +#ifdef NIGHTLY_BUILD > +pref("layout.accessiblecaret.enable_on_touch", true); Nit: Please use layout.accessiblecaret.enabled_on_touch (extra 'd') for consistency with the original 'enabled' pref.

I made the changes you pointed out, and also modified testing/profiles/prefs_general.js to set the new pref to false. In automation touch events are force-enabled on all platforms (even OS X) and so it forces on the accessible caret everywhere, which we don't want because it causes loads of test failures [1]. So setting the pref to false in the testing prefs ensures that doesn't happen. Try push with those changes is at [2] and is looking pretty good so far. [1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7df7542e7eb [2] https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b508a4df47d

Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/06e0bfe5648b On desktop, allow the context menu to pop up concurrently with text selection. r=tylin https://hg.mozilla.org/integration/mozilla-inbound/rev/75cf79c359aa Add a new pref to enable the accessible carets if touch events are supported, and enable the pref on nightly. r=tylin

Hm, the try push completed and is showing 4 consistent reftest failures on Android. Those jobs haven't finished on inbound yet but it's quite likely the failures are from my patches and will result in a backout. I'm gonna try reproducing locally to see what's going on.

Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/31d6ebea6244 Follow-up to keep the accessible caret disabled on some reftests that require it. r=me on a CLOSED TREE

https://hg.mozilla.org/mozilla-central/rev/06e0bfe5648b https://hg.mozilla.org/mozilla-central/rev/75cf79c359aa https://hg.mozilla.org/mozilla-central/rev/31d6ebea6244