Closed
Bug 1119126
Opened 9 years ago
Closed 9 years ago
[TextSelection] The paste icon overlaps keyboard if tapping caret quickly before keyboard shows up
Categories
(Core :: DOM: Selection, defect, P2)
Tracking
()
People
(Reporter: gchang, Assigned: mtseng)
References
Details
Attachments
(3 files, 8 obsolete files)
46.11 KB,
image/png
|
Details | |
46 bytes,
text/x-github-pull-request
|
alive
:
review+
|
Details | Review |
3.44 KB,
patch
|
roc
:
review+
bajaj
:
approval-mozilla-b2g37+
|
Details | Diff | Splinter Review |
### 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•9 years ago
|
Blocks: CopyPasteLegacy
QA Whiteboard: [textselection]
Reporter | ||
Comment 1•9 years ago
|
||
Comment 2•9 years ago
|
||
Hi Morris, gaia need a event to update position in shortcut mode after resizing...
Flags: needinfo?(mtseng)
Comment 3•9 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)
Comment 4•9 years ago
|
||
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)
Updated•9 years ago
|
Assignee: nobody → gduan
Comment 5•9 years ago
|
||
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+
Comment 6•9 years ago
|
||
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
Closed: 9 years ago
Resolution: --- → FIXED
Comment 7•9 years ago
|
||
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 → ---
Assignee | ||
Updated•9 years ago
|
Priority: -- → P2
Assignee | ||
Comment 9•9 years ago
|
||
Assignee | ||
Comment 10•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8552935 -
Flags: review?(roc)
Assignee | ||
Updated•9 years ago
|
Attachment #8552935 -
Flags: review?(roc)
Assignee | ||
Comment 11•9 years ago
|
||
Attachment #8552935 -
Attachment is obsolete: true
Attachment #8552945 -
Flags: review?(roc)
Attachment #8552945 -
Flags: review?(roc) → review+
Assignee | ||
Comment 12•9 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a5ac1e7a22b5
Updated•9 years ago
|
Component: Gaia::System → Selection
Product: Firefox OS → Core
Version: unspecified → Trunk
Assignee | ||
Comment 14•9 years ago
|
||
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
Assignee | ||
Comment 17•9 years ago
|
||
try https://treeherder.mozilla.org/#/jobs?repo=try&revision=f8037cca6072
Assignee | ||
Comment 18•9 years ago
|
||
Fix build failed. try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9d1717398813
Attachment #8557766 -
Attachment is obsolete: true
Attachment #8557778 -
Flags: review?(roc)
Assignee | ||
Comment 19•9 years ago
|
||
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-
Assignee | ||
Comment 21•9 years ago
|
||
Use script runner to dispatch event.
Attachment #8557778 -
Attachment is obsolete: true
Assignee | ||
Comment 22•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ddb2ebd4ea66
Assignee | ||
Comment 23•9 years ago
|
||
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
Assignee | ||
Comment 24•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ec7cef9058d
Assignee | ||
Updated•9 years ago
|
Attachment #8558340 -
Flags: review?(roc)
Attachment #8558340 -
Flags: review?(roc) → review+
Reporter | ||
Updated•9 years ago
|
QA Whiteboard: [textselection] → [COM=Text Selection]
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•9 years ago
|
Attachment #8552936 -
Attachment is obsolete: true
Assignee | ||
Comment 25•9 years ago
|
||
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?
Comment 26•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a372a90a9475
Keywords: checkin-needed
Comment 27•9 years ago
|
||
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)
Assignee | ||
Comment 29•9 years ago
|
||
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
Comment 30•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/78f919c2fb5a
Keywords: checkin-needed
Comment 31•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/78f919c2fb5a
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
status-firefox38:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Updated•9 years ago
|
Attachment #8558340 -
Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
Comment 32•9 years ago
|
||
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
Comment 33•9 years ago
|
||
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?]
Flags: needinfo?(ktucker)
Updated•9 years ago
|
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.
Description
•