Closed
Bug 1121586
Opened 9 years ago
Closed 9 years ago
[Dialer][Text Selection] Hitting the Delete button will remove the digit from the end of a string of entered digits instead of from where the cursor is currently located.
Categories
(Firefox OS Graveyard :: Gaia::Dialer, defect)
Tracking
(blocking-b2g:2.2+, b2g-v2.2 verified, b2g-master verified)
People
(Reporter: jmitchell, Assigned: salva)
References
()
Details
(Whiteboard: [3.0-Daily-Testing])
Attachments
(4 files)
Description: When in the dialer app (dial pad) you can tap in the middle of a string of entered digits to move the cursor to that position. When you hit the delete key the typical expectation will be to delete the digit in the position to the left of the cursor. Instead; the digit at the very end of the string of digits is deleted. This would prevent the user from only changes a single mistyped number in the middle or beginning of the string without having to delete the entire string. This is also different than other market devices which follows the expected results of deleting the digit to the left of the cursor (Verified on Android ZTE Savvy) Repro Steps: 1) Update a Flame to 20150114010205 2) Launch Dialer 3) Type in 6 or 7 digits 4) Tap in the middle of the string of digits to move the cursor to there 5) Hit the delete button Actual: The # at the end of the string is deleted Expected: The # to the left of the cursor will be deleted Environmental Variables: Device: Flame Master Build ID: 20150114010205 Gaia: e2a0f7c311119d4a8e160bdfb9e28a0e61a180fc Gecko: 63006936ab99 Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76 Version: 38.0a1 (Master) Firmware Version: V18d-1 User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0 Repro frequency: 7/7 See attached: logcat - video clip: http://youtu.be/VOi8ohi2zPM ------------------------------------------------------------------------------- This also occurs on 2.2 (V18d-1) and (V18d) Text Selection in not a feature prior to 2.2 Device: Flame 2.2 (KK - Nightly - Full-Flashed) Build ID: 20150114002502 Gaia: 7c5b27cad370db377b18a742d3f3fdb0070e899f Gecko: 748b20315f75 Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76 Version: 37.0a2 Firmware Version: V18d-1 User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0 Device: Flame 2.2 (KK - Nightly - Full-Flashed) Build ID: 20150114002502 Gaia: 7c5b27cad370db377b18a742d3f3fdb0070e899f Gecko: 748b20315f75 Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76 Version: 37.0a2 Firmware Version: V18d User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Reporter | ||
Updated•9 years ago
|
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
Updated•9 years ago
|
Comment 2•9 years ago
|
||
[Blocking Requested - why for this release]: The UI looks confusing now that the Text Selection feature is enabled. A user may think that this new feature is broken. Carrie, can we do something to help to make things more clear?
blocking-b2g: --- → 2.2?
Flags: needinfo?(cawang)
Updated•9 years ago
|
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][COM=Gaia::Dialer]
Comment 3•9 years ago
|
||
triage: very confusing to end user. Carrie, please advise the correct behavior
blocking-b2g: 2.2? → 2.2+
Comment 4•9 years ago
|
||
Hi, If user locates the cursor in the middle of the numbers, we shall always delete the number from where the cursor is. If the user didn't locate the cursor, we shall delete the numbers from the end of the input digits. Thanks!
Flags: needinfo?(cawang)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → salva
Assignee | ||
Comment 5•9 years ago
|
||
Hi Gabriel, Do you mind to take a look at the patch to check if you feel comfortable with the proposed solution, please? I'm going to write a comment now pointing out all the issues I found while addressing this bug. Thank you!
Attachment #8552577 -
Flags: feedback?(gsvelto)
Assignee | ||
Comment 6•9 years ago
|
||
Hello. While fixing this bug I found some other problems noteworthy: 1- Adding digits at the cursor position: in the same way we have a problem deleting characters at the cursor position, we have another problem related with adding more digits. Once the users alter the cursor position, new taps on the keys always add the digits at the end. Although I think this should be a different bug. 2- Can not paste in the dialer: just that, you can select, copy but not paste. 3- Can not force cursor to be shown: with the provided patch, even if we fix this bug, each time the delete key is tapped, the cursor dissapear. It's still there, and you can delete the previous character but it's not shown. This could led the user to confussion. Asking UX for the 3 items above. I would want to confirm both UX and me see some problems here. Asking Etienne for the caret item. Do you know any way to force the cursor to be shown?
Flags: needinfo?(etienne)
Flags: needinfo?(cawang)
Comment 7•9 years ago
|
||
Haven't touch this in a while, redirecting the NI.
Flags: needinfo?(etienne) → needinfo?(drs.bugzilla)
Comment 8•9 years ago
|
||
Comment on attachment 8552577 [details] [review] Tracking the caret position to allow deleting at the current character I'm taking this since Gabriele is on PTO today. (In reply to Salvador de la Puente González [:salva] from comment #6) > 3- Can not force cursor to be shown: with the provided patch, even if we > fix this bug, each time the delete key is tapped, the cursor dissapear. It's > still there, and you can delete the previous character but it's not shown. > This could led the user to confussion. > Asking Etienne for the caret item. Do you know any way to force the cursor > to be shown? This is happening because the phone number field is losing focus. It can be fixed by setting a 'focus' event listener on the delete button, and then returning focus to the phone number field. This otherwise seems reasonable to me.
Flags: needinfo?(drs.bugzilla)
Attachment #8552577 -
Flags: feedback?(gsvelto) → feedback+
Assignee | ||
Comment 9•9 years ago
|
||
Hi Dough, what do you prefer for item 1:
> 1- Adding digits at the cursor position: in the same way we have a problem
> deleting characters at the cursor position, we have another problem related
> with adding more digits. Once the users alter the cursor position, new taps
> on the keys always add the digits at the end. Although I think this should
> be a different bug.
Should we file a different bug or reword this one to include this case?
Flags: needinfo?(drs.bugzilla)
Assignee | ||
Comment 10•9 years ago
|
||
s/Dough/Doug/
Comment 11•9 years ago
|
||
(In reply to Salvador de la Puente González [:salva] from comment #9) > Should we file a different bug or reword this one to include this case? Please file a new bug for that and set "See Also: bug 11121586" and "blocking-b2g: 2.2?".
Flags: needinfo?(drs.bugzilla)
Comment 12•9 years ago
|
||
I may need some input from keyboard UX here.
Flags: needinfo?(cawang) → needinfo?(ofeng)
Assignee | ||
Comment 13•9 years ago
|
||
Comment on attachment 8552577 [details] [review] Tracking the caret position to allow deleting at the current character Hi Doug, do you mind reviewing the patch? Thank you.
Attachment #8552577 -
Flags: review?(drs.bugzilla)
Assignee | ||
Comment 14•9 years ago
|
||
By the way, related with the UX, I discovered another not covered case. Now, if we keep pressed the delete key, we clear the whole number. What if the cursor is set in the middle of the number? In case of a long press, what should we do? I was thinking about clear from the beginning to the cursor's position bu I think we need UX input here.
Comment 16•9 years ago
|
||
Comment on attachment 8552577 [details] [review] Tracking the caret position to allow deleting at the current character Redirecting to Gabriele. At the very least, this needs: 1. Tests. 2. The same changes to be copied over to the Emergency Call app.
Attachment #8552577 -
Flags: review?(drs.bugzilla) → review?(gsvelto)
Assignee | ||
Comment 17•9 years ago
|
||
I'm going to prepare the tests, Gabriel. I don't know what is your policy here but it seems to me this needs integration tests more than unit ones. Even so, I can provide unit tests for the conversions between caret position and insert position if you want. Regarding integration tests, do you know if there is possible to say for an input: "click before the nth character"?
Flags: needinfo?(gsvelto)
Comment 18•9 years ago
|
||
(In reply to Salvador de la Puente González [:salva] from comment #17) > I'm going to prepare the tests, Gabriel. I don't know what is your policy > here but it seems to me this needs integration tests more than unit ones. I agree that integration tests would be better for this scenario. > Regarding integration tests, do you know if there is possible to say for an > input: "click before the nth character"? I'm not sure about that, Johan might know more so I'm needinfo'ing him. I've come across at least one test that is explicitly setting `selectionStart' and `selectionEnd' so that might work too: https://github.com/mozilla-b2g/gaia/blob/6318d7923760d071bd1faa5258e5244777b5d33e/tests/python/gaia-ui-tests/gaiatest/tests/functional/keyboard/test_keyboard_bug_1073870.py#L32
Flags: needinfo?(gsvelto) → needinfo?(jlorenzo)
Comment 19•9 years ago
|
||
I don't think there is a way to make a selection thanks to Marionette. Do you agree, Malini? If that's the case, I would use the same piece of JS than Gabriele pointed out. One note though compared to it, this piece of JS seems generic enough to be placed at a higher level than the test case itself.
Flags: needinfo?(jlorenzo) → needinfo?(mdas)
Comment 20•9 years ago
|
||
Comment on attachment 8552577 [details] [review] Tracking the caret position to allow deleting at the current character This is looking good to me save for a few small changes I've described in the PR. I'm r- it mostly because it doesn't have tests yet (I'd f+ it but :drs did it first ;-)). With my comments addressed and the appropriate tests I'd say this should be good to land.
Attachment #8552577 -
Flags: review?(gsvelto) → review-
Comment 21•9 years ago
|
||
After discussion with Carrie, we think the UX should be: 1. When tapping on the digit area, there should be a cursor, and user can move it to adjust the insertion/deletion point. 2. Long press on the digit area (when the digit area is empty) or tap the touch caret, there should be a utility bubble with Paste button ONLY. 3. User CANNOT select text. However, after discussion with George, there seems to be some limitation for now. George, could you explain it?
Flags: needinfo?(ofeng)
Flags: needinfo?(gduan)
Flags: needinfo?(cawang)
Assignee | ||
Comment 22•9 years ago
|
||
Regarding the original items with need for UX input in comment 6: > 1- Adding digits at the cursor position: in the same way we have a problem > deleting characters at the cursor position, we have another problem related > with adding more digits. Once the users alter the cursor position, new taps > on the keys always add the digits at the end. Although I think this should > be a different bug. This is being addressed in bug 1126538. > 2- Can not paste in the dialer: just that, you can select, copy but not > paste. With the UX clarification, now it turns out to not allow to copy but allow to paste. I usually use the dialer to take a quick numerical note. It's a pity to loose copy functionallity. Moreover when there are more problems implementing paste than implementing copy. > 3- Can not force cursor to be shown: with the provided patch, even if we > fix this bug, each time the delete key is tapped, the cursor dissapear. It's > still there, and you can delete the previous character but it's not shown. > This could led the user to confussion. This is already solved. We still need input about what should happen when long pressing the delete button if the cursor is not at the end of the input. See comment 14.
Flags: needinfo?(ofeng)
Comment 23•9 years ago
|
||
Hi Salvador, For comment 14, if users set the cursor in the middle of the digits and keep pressing the delete button, it will clear from the digit which is on the left side of the cursor to the beginning. For example, 1234|555 user long press the delete button it will delete from 4 to 1 Thanks!
Comment 24•9 years ago
|
||
According to comment 22, we do think that "Copy" is quite convenient in some use cases. Our expectation is that if user long taps on the numbers and the numbers will be selected with a copy option, then user can copy the numbers to do some further edit in other APPS. However, we don't have API to do only "Copy" for now, if we want to provide the function here in Dialer, the option "copy" will show up with two handles and the utility bubble (copy/ paste/ cut/ select all) as in general edit field. This will make the behavior in Dialer look weird. It's like "edit" is the highest priority in Dialer and makes inputting digits become complicated. Hence, we'd suggest remove it for now to keep things simple until the API is ready. Thanks!
Flags: needinfo?(ofeng)
Comment 25•9 years ago
|
||
(In reply to Omega Feng [:Omega] [:馮於懋] (please ni?) from comment #21) > After discussion with Carrie, we think the UX should be: > 1. When tapping on the digit area, there should be a cursor, and user can > move it to adjust the insertion/deletion point. > 2. Long press on the digit area (when the digit area is empty) or tap the > touch caret, there should be a utility bubble with Paste button ONLY. The input field is set 'readonly', we have to remove this attribute to enable paste function and perhaps create some special attribute for system's input manager to decide whether to show the keyboard or not. > 3. User CANNOT select text. AFAK, there's no api or attribute to only enable cursor but disable selection (long-press) function. > > However, after discussion with George, there seems to be some limitation for > now. George, could you explain it?
Flags: needinfo?(gduan)
Assignee | ||
Comment 26•9 years ago
|
||
Comment on attachment 8552577 [details] [review] Tracking the caret position to allow deleting at the current character Gabriele, I'm having problems with integration tests. I don't know why, but the test "Deleting a couplle of digits..." [1] is failing and I don't understand where the problem is. Can you help me, please? It seems the restore caret position function is not being called as the second del tap removes the last digit but it is called indeed after the first del tap. The problem is, by the moment the second tap is performed, selectionStart and selectionEnd are not properly set. [1] https://github.com/mozilla-b2g/gaia/pull/27574/files#diff-f0cc16144fe4eb21f930543ed9412d7aR141
Attachment #8552577 -
Flags: review- → review?(gsvelto)
Assignee | ||
Comment 27•9 years ago
|
||
Mmmm, a doubleTap instead of two consecutive taps fix the problem. Let me know if you feel ok with this solution. if you have a clue, let me know, please.
Comment 29•9 years ago
|
||
Comment on attachment 8552577 [details] [review] Tracking the caret position to allow deleting at the current character This is looking good except for a test where you've added an explicit timeout. I've left a comment on the PR.
Assignee | ||
Comment 30•9 years ago
|
||
Comment on attachment 8552577 [details] [review] Tracking the caret position to allow deleting at the current character I finished to squish all the changes the PR is ready.
Updated•9 years ago
|
Status: NEW → ASSIGNED
Comment 31•9 years ago
|
||
Comment on attachment 8552577 [details] [review] Tracking the caret position to allow deleting at the current character Excellent, thanks. Remember to add r=gsvelto to the commit message before landing.
Attachment #8552577 -
Flags: review?(gsvelto) → review+
Assignee | ||
Comment 32•9 years ago
|
||
master: 1b73c61286410034804146b947f4a6ef2ff6d880 Notice as, per comment 25, we cannot implement the behaviour recommended by UX in comment 21.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Updated•9 years ago
|
Target Milestone: --- → 2.2 S5 (6feb)
Assignee | ||
Comment 33•9 years ago
|
||
Comment on attachment 8552577 [details] [review] Tracking the caret position to allow deleting at the current character [Approval Request Comment] [Bug caused by] (feature/regressing bug #): bug 1092427 [User impact] if declined: misleading and annoying behaviour in the dialer input flow for deleting digits. [Testing completed]: yes [Risk to taking this patch] (and alternatives if risky): moderate [String changes made]: none
Attachment #8552577 -
Flags: approval-gaia-v2.2?
Updated•9 years ago
|
Attachment #8552577 -
Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Comment 34•9 years ago
|
||
v2.2: https://github.com/mozilla-b2g/gaia/commit/1e49935471ad6c54969cfa86066e2c6e1256393e
Comment 35•9 years ago
|
||
This bug verified successfully on flame 2.2 Build ID 20150208002500 Gaia Revision e827781324cbde91d2434b388f5dead3303a85ee Gaia Date 2015-02-06 20:54:14 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/0552759956d3 Gecko Version 37.0a2 Device Name flame Firmware(Release) 4.4.2 Refer to video VIDEO0280.mp4
Comment 36•9 years ago
|
||
Comment 37•9 years ago
|
||
This bug has been successfully verified on latest Flame v3.0. See attachment: verified_v3.0.mp4 Reproduce rate: 0/5 Flame 3.0 build: Build ID 20150208010208 Gaia Revision 994896b89ba10cad39a87f90eeaba9ae5e17c3a6 Gaia Date 2015-02-07 19:22:02 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/be65d1fde126 Gecko Version 38.0a1 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150208.050436 Firmware Date Sun Feb 8 05:04:47 EST 2015 Bootloader L1TC000118D0
Comment 38•9 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•