[Text Selection] migrate touchcarettap handler to selectionstatechange

RESOLVED FIXED in 2.2 S6 (20feb)

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: gduan, Assigned: gduan)

Tracking

unspecified
2.2 S6 (20feb)
x86
macOS
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.2 fixed, b2g-master fixed)

Details

Attachments

(2 attachments)

Gecko from bug 1120750 will handle touchcarettap event in selectionstatechange. We also need to modify gaia and add test before bug 1120750 is landing.
Assignee: nobody → gduan
There's still issue after Gecko with bug 1119126 and bug 1120750.

When focused element may blocked by keyboard, gecko help us to auto scroll the element into visible field. In that case, bubble triggered by singletap won't update its position. We need to figured how to fix this issue.
Flags: needinfo?(mtseng)
Posted file PR to master
Hi Morris,
could I have your feedback on this patch?
Basically, I've separated collapse and selection mode here.
Attachment #8557802 - Flags: feedback?(mtseng)
Comment on attachment 8557802 [details] [review]
PR to master

Looks good.
Flags: needinfo?(mtseng)
Attachment #8557802 - Flags: feedback?(mtseng) → feedback+
Comment on attachment 8557802 [details] [review]
PR to master

Hi Morris,
I've modified a little bit, so could I have your feedback again before landing.
This gaia might break some use cases with current master
1. when keyboard shows up, the bubble is still there
2. long press empty column will have selectAll button

So, we'd better make sure the two gecko are ready.
Attachment #8557802 - Flags: feedback+ → feedback?(mtseng)
Comment on attachment 8557802 [details] [review]
PR to master

Looks great!! Some comments on github. Thanks
Attachment #8557802 - Flags: feedback?(mtseng) → feedback+
Comment on attachment 8557802 [details] [review]
PR to master

Hi Alive,
could I have your review on this patch?
This patch need to be merged before bug 1119126 and bug 1120750 which migrate touchoncaret event into selectionstatechange and send new position event while frame is resized (currently we listen to system-resize as a workaround).

I also separate collpased handler from selection handler.
Attachment #8557802 - Flags: review?(alive)
Comment on attachment 8557802 [details] [review]
PR to master

r+ with nits
Attachment #8557802 - Flags: review?(alive) → review+
Thanks, master
https://github.com/mozilla-b2g/gaia/commit/827389929cc20dda5ade6e887d99894905f4f7b9
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
This gaia patch blocks bug 1119126 and bug 1120750.
blocking-b2g: --- → 2.2?
Comment on attachment 8561176 [details] [review]
[PullReq] cctuan:1127207-v22 to mozilla-b2g:v2.2

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Not a regression nor feature.
[User impact] if declined:
This is gaia patch for bug 1120750 and bug 1119126, without its landing, we cannot solve below issues.
bug 1120750: User impact if declined: The position of shortcut bubble for copy/paste would be wrong when it appears.
bug 1119126:
Copy/paste bubble position may be wrong after scroll or reflow.

[Testing completed]: Yes, manual test and unit test.
[Risk to taking this patch] (and alternatives if risky):
No. However this gaia patch might break some marionette test before bug 1120750 and bug 1119126 are merged.
[String changes made]:
Attachment #8561176 - Flags: approval-gaia-v2.2?

Updated

4 years ago
blocking-b2g: 2.2? → 2.2+
Please request Gaia v2.2 approval on this when you get a chance.
Flags: needinfo?(gduan)
Target Milestone: --- → 2.2 S6 (20feb)
Thanks for reminding, I already write it in comment 11.
Flags: needinfo?(gduan)
Attachment #8561176 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Blocks: 1202541
You need to log in before you can comment on or make changes to this bug.