1052424 - [Messages] Cursor should be in the "to" field by default while forwarding messages

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::SMS, defect)

Description

[ankit93040]

User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/36.0.1985.125 Safari/537.36

Steps to reproduce:

Pre-condition - Have a message thread.
1. Go to message->thread
2. click on any message text -> forward


Actual results:

Cursor first goes to TO field then comes back to Message filed. The cursor comes at first place in text field.


Expected results:

cursor should wither remain in TO field or should be appended at the end of the text content

Comment 1

[Julien Wajsberg [:julienw]]

I think we have another bug for showing the cursor at the end of the text content, but can't find it now.

Can you please give your gaia/gecko hashes?

Comment 2

[ankit93040]

gaia  : master
gecko  :6a137bbc4cc39a246c7e4de7e358117400c80f26

Comment 3

[Julien Wajsberg [:julienw]]

If I refer the document in [1] (page 15) then it should be in the "To" document and the fact that it moves to the Compose area is a bug. However, in my phone with v1.3 it seems we already do the wrong behavior.

Hey Jenny, do you confirm the expected behavior here?

Comment 4

[Jenny Lee]

Hey Julien, can you provide the link to the document you were referring to? Thanks!

Agree that cursor should be in the "to" field.

Comment 5

[Julien Wajsberg [:julienw]]

ahah sorry.

[1] https://mozilla.app.box.com/s/0u4jt353ei9ov2c150ip/1/1170795225/11918301182/1

Comment 6

[Jenny Lee]

Ahh thanks! Let's follow the spec specified behavior ;)

Comment 7

[Luke Chang [:lchang] (inactive)]

I'd like to take a look at this issue.

Comment 8

[Luke Chang [:lchang] (inactive)]

Modify title according to the discussion above.

Comment 9

[Luke Chang [:lchang] (inactive)]

Attachment: Pull Request 23023

Hi Steve,

Another patch needs your help. Thanks.

Comment 10

[Steve Chung [:steveck]]

Comment on attachment 8475013 [details] [review]
Pull Request 23023

Hi Luke, thanks for the patch, but please be careful about the activity path(with new number added), the focus would be set to the to field but the wrong place, you may need to focus again to the to field in this case. But I would like to ni UX about adding new number case first.

Hi Jenny, if we are sending the message from contact app, should we focus on the message input field? Or we should sync the behavior that always focus on to fields?

Comment 11

[Jenny Lee]

The two use cases you described are not the same so I think they should have different behavior. When sending message via contact app, cursor should be focused on composer. Thanks!

Comment 12

[ankit93040]

Hi Luke

Greetings!

Since, you have already uploaded the pull request. Just a few things pending. Will you please make it to completion or may be I can help you with as in I want to get this fixed for myself.

Thanks!

Comment 13

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8475013 [details] [review]
Pull Request 23023

(In reply to ankit93040 from comment #12)
> Since, you have already uploaded the pull request. Just a few things
> pending. Will you please make it to completion or may be I can help you with
> as in I want to get this fixed for myself.

Thanks for your concern and sorry for the late reply. I was busy dealing with another bugs for the past few days. Even so, I've already addressed Jenny's comment and updated my pull request so let's move on. :)


Steve, would you mind reviewing it again? Thanks a lot!

Comment 14

[Steve Chung [:steveck]]

Hi Luke, per our discussion I think we could try to fix the compose handling scenario into better shape. Please refine this part and request review again, thanks!

Comment 16

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8475013 [details] [review]
Pull Request 23023

Hi Steve,

My patch is updated base on our discussion. Thanks.

Comment 17

[Steve Chung [:steveck]]

Comment on attachment 8475013 [details] [review]
Pull Request 23023

Except for the test refinement, I still have some concern in handleDraft. Hi julien, since we only call handleDraft for entering compose panel called path, is it possible that we need to handle draft but no draft id provided at first?

Comment 18

[Julien Wajsberg [:julienw]]

answered on github: currently nope, but maybe in the future, so we might try to be future-proof here.

Comment 19

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8475013 [details] [review]
Pull Request 23023

Hi Julien,

Thanks for your suggestions.


Hi Steve,

I've addressed the comments. Please take a look. Thanks.

Comment 20

[Luke Chang [:lchang] (inactive)]

Hi Steve,

The original patch seems to conflict with latest master branch so I've updated my pull request.

Comment 21

[Steve Chung [:steveck]]

Comment on attachment 8475013 [details] [review]
Pull Request 23023

Just one comment on github, could you please check again? Thanks.

Comment 22

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8475013 [details] [review]
Pull Request 23023

Hi Steve,

Thanks for your comment. It has been addressed.

Comment 23

[Steve Chung [:steveck]]

Comment on attachment 8475013 [details] [review]
Pull Request 23023

Looks good, thanks!

Comment 24

[Luke Chang [:lchang] (inactive)]

Steve, Thanks for reviewing.


Gaia-Try passed: https://tbpl.mozilla.org/?rev=d196c328789e0515e698d852dfbc9453ab5a0406&tree=Gaia-Try

landed on master: https://github.com/mozilla-b2g/gaia/commit/a05f5447bb9ce4e5d29280a9c5dc60671effd6d7

Comment 27

[Kai-Zhen Li [:seinlin][:kli]]

Merge into 2.0/2.0m got conflicts.

Comment 28

[Luke Chang [:lchang] (inactive)]

Attachment: pull request to v2.0m

Waiting for gaia-try...

Comment 29

[Luke Chang [:lchang] (inactive)]

There seems to be no gaia-try hook on v2.0m. Check in since local test is passed.

landed on v2.0m: https://github.com/mozilla-b2g/gaia/commit/a37ffb70612c77bcd0bfeef1e38a41dac73f0eda

Comment 30

[Josh Cheng [:josh]]

Hi Norry,
Please reproduce on Flame 2.1 and verify on Woodduck 2.0M. Thanks!

Comment 31

[Duane Dixon [:ddixon]]

Issue DOES occur in Flame 2.1 build (Full Flash, nightly). 

Actual Results: Cursor initially appears in "To:" field then transitions to the start of the text content field (as described in Comment 0) when user attempts to forward a text message.  

Device: Flame 2.1
Build ID: 20141022001201
Gaia: 3d9cc667f4e929861a9a77c41096bbf5a9c1bde0
Gecko: 928b18f7d8ff
Version: 34.0 (2.1)
Firmware Version: v188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 32

[Julien Wajsberg [:julienw]]

Just want to notice that the master patch here depends on Lana's patch in bug 959201, and Luke's patch in bug 1043682. That's why there are conflicts when uplifting this patch only.

Comment 33

[Eric]

It does not exist on woodduck 2.0M
======================================
Test bulid information:
Gaia-Rev        fdb8236d7d99061ef6bedc021fd6b482e1af3f5a
Gecko-Rev       3d95c1683ef5eb017bd15bc38ba111ddb1ad792e
Build-ID        20141024050313
Version         32.0
Device-Name     soul35
FW-Release      4.4.2
FW-Incremental  1414098327
FW-Date         Fri Oct 24 05:05:49 CST 2014

Comment 34

[helen（leave from Mozilla）]

Attachment: Flame_v2.2.3gp

This bug has been verified as pass on latest build of Flame v2.2 by the STR in Comment 0. 

Results:
Cursor remains in the "to" field by default while forwarding messages.
 
See attachment: Flame_v2.2.3gp
Reproduce rate: 0/10

Device: Flame v2.2(Verified)
Build ID               20150709162504
Gaia Revision          84d0c76370dcd3d25813b00de55194730884355b
Gaia Date              2015-07-09 13:09:14
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/e002005dc994
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150709.202124
Firmware Date          Thu Jul  9 20:21:35 EDT 2015
Bootloader             L1TC000118D0

Comment 35

[Lancy(Leave from Mozilla)]

Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/11075/