[TextSelection] The paste icon overlaps keyboard if tapping caret quickly before keyboard shows up

VERIFIED FIXED in Firefox 38, Firefox OS v2.2

Status

()

P2
normal
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: gchang, Assigned: mtseng)

Tracking

Trunk
mozilla38
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+, firefox36 wontfix, firefox37 wontfix, firefox38 fixed, b2g-v2.2 verified, b2g-master verified)

Details

Attachments

(3 attachments, 8 obsolete attachments)

46.11 KB, image/png
Details
46 bytes, text/x-github-pull-request
alive
: review+
Details | Review | Splinter Review
3.44 KB, patch
roc
: review+
Details | Diff | Splinter Review
(Reporter)

Description

4 years ago
### Steps:
1. Open Messages app
2. Click pencil icon on the upper-right corner to create a new message
3. Type characters in the message body input field
4. Long press on any character to trigger utility bubble
5. Tap copy icon
6. Tap other area to make keyboard disappeared
7. Tap the input field again
8. When the caret (water drop) shows, tap the water drop right away
9. The paste icon will overlap keyboard

### Expected:
1. The paste icon should not overlap keyboard

### Actual:
1. The paste icon overlaps keyboard

### Reproduce rate
always

### Version:
Gaia-Rev        d4dac29613076bdba3cb8adc217deadb08a2ac20
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/70de2960aa87
Build-ID        20150107160220
Version         37.0a1
Device-Name     flame
FW-Release      4.4.2
(Reporter)

Updated

4 years ago
Blocks: 1023688
QA Whiteboard: [textselection]
(Reporter)

Comment 1

4 years ago
Created attachment 8545716 [details]
The image of paste icon overlapping keyboard
Hi Morris,
gaia need a event to update position in shortcut mode after resizing...
Flags: needinfo?(mtseng)

Comment 3

4 years ago
(In reply to George Duan [:gduan] [:喬智] from comment #2)
> Hi Morris,
> gaia need a event to update position in shortcut mode after resizing...
George, as I discussed with Morris, gaia could help to fix this issue. Let us know if you still need gecko's help.
Flags: needinfo?(mtseng) → needinfo?(gduan)
Created attachment 8549339 [details] [review]
PR to master

Hi Alive,
could you review this patch?

Currently gaia handle bubble's displaying when shortcut and single tap mode, so we also need to hide it once window is resized.
Flags: needinfo?(gduan)
Attachment #8549339 - Flags: review?(alive)
Assignee: nobody → gduan
Comment on attachment 8549339 [details] [review]
PR to master

Squash commits please.

Also, is gecko still telling us to show the bubble again after hiding it? IMO only hide seems strange.
Attachment #8549339 - Flags: review?(alive) → review+
Merged to master: https://github.com/mozilla-b2g/gaia/commit/57b771e6976e9ce58a4c058ee00bfcb999c304f4

This is a workaround, however, we still have some cases like iframe's iframe resize with shortcut.
Gecko should try to send the same events queue of cursor event as selection caret. (see bug 1110039)
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Hi Morris,
as offline discuss, in shortcut mode, we still need gecko to send proper event while frame is resized , which is just like selection mode. Or bubble might flash while user tap other type of input column.
Status: RESOLVED → REOPENED
Flags: needinfo?(mtseng)
Resolution: FIXED → ---
Working on this.
Assignee: gduan → mtseng
Flags: needinfo?(mtseng)
Priority: -- → P2
Created attachment 8552935 [details] [diff] [review]
Send updateposition after reflow when selection is collapsed.
Attachment #8552935 - Flags: review?(roc)
Attachment #8552935 - Flags: review?(roc)
Created attachment 8552945 [details] [diff] [review]
Send updateposition after reflow when selection is collapsed v2.
Attachment #8552935 - Attachment is obsolete: true
Attachment #8552945 - Flags: review?(roc)

Comment 13

4 years ago
Triage: feature blocker.
blocking-b2g: --- → 2.2+

Updated

4 years ago
Component: Gaia::System → Selection
Product: Firefox OS → Core
Version: unspecified → Trunk
Created attachment 8557763 [details] [diff] [review]
Send updateposition after reflow when selection is collapsed v3.

Looks like test may failed if we call FlushPendingNotification inside Reflow callback. So, don't call FlushPendingNotification in the reflow callback in this patch.
Attachment #8552945 - Attachment is obsolete: true
Created attachment 8557765 [details] [diff] [review]
Send updateposition after reflow when selection is collapsed v4.

Rebased.
Attachment #8557763 - Attachment is obsolete: true
Created attachment 8557766 [details] [diff] [review]
Send updateposition after reflow when selection is collapsed v5.

Fix typo.
Attachment #8557765 - Attachment is obsolete: true
Created attachment 8557778 [details] [diff] [review]
Send updateposition after reflow when selection is collapsed v6.

Fix build failed.
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9d1717398813
Attachment #8557766 - Attachment is obsolete: true
Attachment #8557778 - Flags: review?(roc)
Roc, would you mind review this patch again per comment 14? Thanks.
Comment on attachment 8557778 [details] [diff] [review]
Send updateposition after reflow when selection is collapsed v6.

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

::: layout/base/SelectionCarets.cpp
@@ +1300,5 @@
>      // bubble in drag mode. So, don't dispatch event here.
>      if (mDragMode == NONE) {
>        DispatchSelectionStateChangedEvent(GetSelection(),
> +                                         SelectionState::Updateposition,
> +                                         false);

This is probably unwise. It's not a good idea to simply not flush and hope the layout is OK.

I suggest instead of putting this in a post-reflow callback, you put it in a scriptrunner. Running script is disabled during reflow, so your scriptrunner will run after reflow in a state where it's ok to flush layout. But you have to be careful not to trigger another reflow that will post another scriptrunner and put you in an infinite loop...
Attachment #8557778 - Flags: review?(roc) → review-
Created attachment 8557818 [details] [diff] [review]
Send updateposition after reflow when selection is collapsed v7.

Use script runner to dispatch event.
Attachment #8557778 - Attachment is obsolete: true
Created attachment 8558340 [details] [diff] [review]
Send updateposition after reflow when selection is collapsed v8.

Unfortunately, add script runner would causes infinite reflow loop... After some experiment, I found that zero range selection also means selection is collapsed. But I really only care about the selection which has range. So I add a check for range count and it looks fine. This patch also send event after scroll so that we can update bubble after scrolling as well.
Attachment #8557818 - Attachment is obsolete: true
Attachment #8558340 - Flags: review?(roc)
(Reporter)

Updated

4 years ago
QA Whiteboard: [textselection] → [COM=Text Selection]
Keywords: checkin-needed
Attachment #8552936 - Attachment is obsolete: true
Comment on attachment 8558340 [details] [diff] [review]
Send updateposition after reflow when selection is collapsed v8.

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Copy/paste bubble position may be wrong after scroll or reflow.
Testing completed: on master 
Risk to taking this patch (and alternatives if risky): low risk.
String or UUID changes made by this patch: Not available.
Attachment #8558340 - Flags: approval-mozilla-b2g37?
sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=6402550&repo=mozilla-inbound
Flags: needinfo?(mtseng)
With latest gaia, it looks good. Can be checkin again.

try https://bugzilla.mozilla.org/show_bug.cgi?id=1119126
Flags: needinfo?(mtseng)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/78f919c2fb5a
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38

Updated

4 years ago
Attachment #8558340 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/9631a35e0d14
status-b2g-v2.2: --- → fixed
status-b2g-master: --- → fixed
status-firefox36: --- → wontfix
status-firefox37: --- → wontfix
(Reporter)

Updated

4 years ago
See Also: → bug 1132810
This issue is verified fixed on Flame 3.0 and 2.2.

Observed behavior: Following STR, the paste icon moves to above the message input field instead of overlapping with keyboard.

Device: Flame 3.0
BuildID: 20150406010204
Gaia: ef61ebbe5de8c2c9fc2a8f74a12455044c3b82e9
Gecko: 4fe763cbe196
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Device: Flame 2.2
BuildID: 20150406002503
Gaia: a6351e1197d54f8624523c2db9ba1418f2aa046f
Gecko: c3335a5d3063
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [COM=Text Selection] → [COM=Text Selection][QAnalyst-Triage?]
status-b2g-v2.2: fixed → verified
status-b2g-master: fixed → verified
Flags: needinfo?(ktucker)
QA Whiteboard: [COM=Text Selection][QAnalyst-Triage?] → [COM=Text Selection][QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.