Closed Bug 1639087 Opened 4 years ago Closed 2 years ago

No magnifier with selecting content text on Android P

Categories

(GeckoView :: General, enhancement, P3)

Unspecified
All
enhancement

Tracking

(relnote-firefox 101+, firefox99 wontfix, firefox100 wontfix, firefox101 fixed)

RESOLVED FIXED
101 Branch
Tracking Status
relnote-firefox --- 101+
firefox99 --- wontfix
firefox100 --- wontfix
firefox101 --- fixed

People

(Reporter: j, Assigned: m_kato)

References

Details

Attachments

(2 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Firefox/78.0

Steps to reproduce:

Select some text in content in some page.
Move a text selection cursor by finger.

Actual results:

Display a magnifier on moving a text selection cursor.

magnifier in Gmail: https://user-images.githubusercontent.com/25442310/67729344-f402e680-f9ad-11e9-8270-a2d716bb7e49.png

Expected results:

No magnifier.

This is not supported behaviour at this time. Putting it on the future features list, unless Fenix wants to prioritise this as a feature, at which time we will do an investigation into feasibility.

Severity: -- → S4
Type: defect → enhancement
Priority: -- → P3
Status: UNCONFIRMED → NEW
Ever confirmed: true

Could this be higher on the priority list please?

Actually, there is no way to get drag point of accessible caret on presscaret event and no event during dragging accessible caret .

TYLin, could we add anything "dragcaret" event in CaretChangedReason , and add more member fields to CaretStateChangedEvent for dragging point? Magnifying class support needs current user's dragging touch point.

@@ -35,9 +38,11 @@ interface CaretStateChangedEvent : Event {
   /* The bounding client rect is relative to the visual viewport. */
...
   readonly attribute boolean selectionEditable;
   readonly attribute DOMString selectedTextContent;
+  readonly attribute long clientX; // for presscaret and dragcaret
+  readonly attribute long clientY; // for presscaret and dragcaret
 };

Or do we already have anything event to get dragging point of accessible caret?

Flags: needinfo?(aethanyc)

If the magnifier widget is designed to follow the user's touch point closely, then it makes sense for gecko to provide the touch point to CaretStateChangedEvent.

I was thinking whether we should only magnify the either end of the selection highlight or the blinking cursor if the selection is collapsed, so that we can synthesize the point from boundingClientRect in CaretStateChangedEvent. However, if we design that way, it seems the magnifier widget will jump suddenly when the selection highlight extended, so I guess this is probably not want we want?

We should also be aware of the performance impact on the front-end side. When dragging a caret, we're processing a lot of event points in layout coordinates, but I guess we will report the point in CSS pixels or device pixels in CaretStateChangedEvent. I'm not sure if front-end can handle massive number of events if we relay every event to CaretStateChangedEvent when dragging a caret.

cc Olli in case there are more concerns in adding touch point to CaretStateChangedEvent. (We need a DOM peer to review the CaretStateChangedEvent.idl change for sure.)

Flags: needinfo?(aethanyc)

However, if we design that way, it seems the magnifier widget will jump suddenly when the selection highlight extended, so I guess this is probably not want we want?

At first, I consider to listen current selection only (but there might not be no way whether we detect that user select start or end). But magnifying support on Chrome (and old iOS) seems to listen drag event of caret, and even if selection isn't updated, the magnifying glass position/render content should be updated by touch point.

We should also be aware of the performance impact on the front-end side.

Yes, but this is during selection by accessible caret only, so I don't think this occurs on huge performance issue for all cases. And I should add a pref for magnifying glass support for low-end device's perf issue.

But magnifying support on Chrome (and old iOS) seems to listen drag event of caret, and even if selection isn't updated, the magnifying glass position/render content should be updated by touch point.

I just test Google Chrome and Gmail app on my Pixel 2 (Android 11). I notice the magnifier follows the touch point closely on horizontal axis even if my finger just moves slightly to the right or to the left. However, the magnifying glass seems moving up or down line by line, that it, it doesn't follow the touch point so closely on vertical axis. If this is the platform convention, maybe we should consider implement something similar.

Yes, but this is during selection by accessible caret only, so I don't think this occurs on huge performance issue for all cases. And I should add a pref for magnifying glass support for low-end device's perf issue.

Yeah, it would be nice to have a pref to control the behavior, which allows users who prefer our old behavior to turn the magnifying glass off.

Hi there, I am new here just curious if this text selection magnifier function will land on firefox android nightly first or everywhere on beta and stable release also ?
Thanks guys.

(In reply to BlogMan from comment #9)

Hi there, I am new here just curious if this text selection magnifier function will land on firefox android nightly first or everywhere on beta and stable release also ?
Thanks guys.

Any Firefox feature (desktop and android) will land on nightly first, and move to beta in 4-week release cycle, and finally to stable in another 4 weeks. See our current planned release schedule https://wiki.mozilla.org/Release_Management/Calendar.

To support magnifying glass on GeckoView, I would like to add dragcaret
event and, clientX and clientX in CaretStateChangedEvent chrome event.

Actually, accessible caret fires presscaret and releasecaret when
accessbile caret is pressed or released. But when dragging this caret, no
chrome event is fired. Since magnifying glass listens to moving this caret,
I would like dargcaret for GeckoView.

Also, Users' dragging point is necessary to set better position of magnifying
glass windows. So I also want client point of dragging point on presscaret
and dragcaret event.

This event and properties are on layout.accessiblecaret.magnifier.enabled=true,
So this can be only for GeckoView.

Assignee: nobody → m_kato
Status: NEW → ASSIGNED

Android P+ supports magnify glass and Chrome already supports it.

When accessible caret events are fired for pressing or dragging it, we show
Android's magnifying glass.

Also, this tests won't be run in task cluster since our CI runs on Android 7.

Depends on D137965

Attachment #9262578 - Attachment is obsolete: true
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/ce0a4c0c3d13
Add dragcaret event by accessible caret. r=TYLin,smaug
https://hg.mozilla.org/integration/autoland/rev/b87210d814b0
Add Magnifying glass support in GeckoView. r=geckoview-reviewers,agi,owlish
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 101 Branch

Release Note Request (optional, but appreciated)
[Why is this notable]: long requested feature
[Affects Firefox for Android]: only
[Suggested wording]: magnifier available on Android 9+ for positioning the cursor in forms on webpages
[Links (documentation, blog post, etc)]:

relnote-firefox: --- → ?

Setting status-firefox100=wontfix because we don’t need to uplift this change to Beta.

Regressions: 1765072

Added to the Fx101 relnotes.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: