1064144 - [SMS]Cursor behaviour in case of saved draft

Status: VERIFIED 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/37.0.2062.103 Safari/537.36

Steps to reproduce:

Have a saved draft in message app with some text content .
Go to message->select a draft->See the cursor position


Actual results:

Cursor always shown in first position(in message field)


Expected results:

Need Jenny's feedback on this.
There can be several cases perhaps -
1. When the draft has only saved no.
2. When the draft has only saved content.
3. When the draft has both no & Message content.

Comment 1

[ankit93040]

There are several bugs already for cursor position but I doubt we have a duplicate of this.

Comment 2

[Steve Chung [:steveck]]

Yes, we already have bug 1052424 for the cursor case, but that issue did not focus on variety for draft scenario like you listed below

> 
> Expected results:
> 
> Need Jenny's feedback on this.
> There can be several cases perhaps -
> 1. When the draft has only saved no.
  Focus on message input
> 2. When the draft has only saved content.
  Focus on recipient
> 3. When the draft has both no & Message content.
  Focus on message input

This is the result after some discussion with Jenny, but it would be great if she could confirm the behavior here.

Comment 3

[ankit93040]

Jenny,

From comment#2, point#3 focus on message input (where) ? I mean the cursor should be where in the message input?

I think it should be at the end of content & not at the first position

Comment 4

[Jenny Lee]

See the following for cursor focus:

1. When the draft has only saved recipient - focus cursor on composer.
2. When the draft has only saved content - focus cursor on recipient field.
3. When the draft has both recipient & Message content - focus cursor on composer. 

And yes cursor should be at the end of content (both recipient filed and composer).

Thanks!

Comment 5

[Julien Wajsberg [:julienw]]

Current behavior is we always focus on the composer.

So:

1. When the draft has only saved recipient - focus cursor on composer. -> OK
2. When the draft has only saved content - focus cursor on recipient field. -> KO
3. When the draft has both recipient & Message content - focus cursor on composer. -> OK

Marking as mentored bug.

The interesting code is [1] in ConversationView and [2] in InboxView. That means that currently we always focus the composer when opening a draft.

I think we should change "focusComposer" to a "focus:" property having 3 values: "composer", "recipients", undefined or null. Undefined or null would mean using the logic from comment 4, with a 4th case: "when the draft is empty, or there is no draft, focus cursor on recipients" (this is what happens when we tap the "new message" icon in Inbox).

From what I've seen in the current code, I think the "auto" algorithm would work in all cases where "focusComposer" is not specified right now, and would maybe even allow to remove "focusComposer" in most cases it's currently specified.

We wouldn't use "focus: 'recipients'" yet, but I think this would be used in bug 1087933, if we replace the current "focus recipients" by an automatic algorithm..

Therefore, Augustin, maybe you'd like to take this bug before finishing bug 1087933 ?


[1] https://github.com/mozilla-b2g/gaia/blob/f9f000b23c0f02aa326600a0fa7b1f3690696004/apps/sms/views/conversation/js/conversation.js#L622-L626
[2] https://github.com/mozilla-b2g/gaia/blob/f9f000b23c0f02aa326600a0fa7b1f3690696004/apps/sms/views/inbox/js/inbox.js#L289-L292

Comment 6

[Augustin Trancart [:autra]]

I knew I woudn't be able to land bug 1087933 that easily, sigh :-D

Don't hesitate to steal it from me if I'm taking too much time though.

Comment 7

[Augustin Trancart [:autra]]

So to elaborate on comment #5, the logic can be rewritten as
- if args.focus is defined and is either 'composer' or 'recipients', then honor it.
- else if there are valid recipients, then focus composer 
- else focus recipients field.

Comment 8

[GitHub Autolander Bot]

Attachment: [gaia] autra:bug-1064144 > mozilla-b2g:master

Comment 9

[Augustin Trancart [:autra]]

Comment on attachment 8657609 [details] [review]
[gaia] autra:bug-1064144 > mozilla-b2g:master

Julien, please r? while I'm rebasing the other one ;-)

Thanks!

Comment 10

[Julien Wajsberg [:julienw]]

Comment on attachment 8657609 [details] [review]
[gaia] autra:bug-1064144 > mozilla-b2g:master

I'm _really_ sorry, a big patch landed today, that is touching the draft logic (we removed the "save draft" dialog in bug 1176976). As a result you need to do a rebase :/

I don't think it will be too painful though.

Comment 11

[Augustin Trancart [:autra]]

Comment on attachment 8657609 [details] [review]
[gaia] autra:bug-1064144 > mozilla-b2g:master

Hey Julien,

No worries, I'm now a rebase wizard :-)

flipping r? again, before bitrotting again.

Comment 12

[Julien Wajsberg [:julienw]]

Comment on attachment 8657609 [details] [review]
[gaia] autra:bug-1064144 > mozilla-b2g:master

I left some simple comments but overall it works ok and I see all cases working as before except the one case we want to fix here.

Please put your changes in a separate commit and flip the r? flag when you're ready :)

Comment 13

[Augustin Trancart [:autra]]

Comment on attachment 8657609 [details] [review]
[gaia] autra:bug-1064144 > mozilla-b2g:master

Hey Julien, I'm ready :-) Please r? again.

Comment 14

[Julien Wajsberg [:julienw]]

Comment on attachment 8657609 [details] [review]
[gaia] autra:bug-1064144 > mozilla-b2g:master

r=me

let's land this !

Please squash, rebase, wait for a green try, and request checkin-needed -- unless you can land by yourself now :)

Comment 15

[Augustin Trancart [:autra]]

\o/ Yay!

Comment 16

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/gaia/commit/fabee5e040ea719f7f8423dc57ab5a01e4745eb6

Comment 17

[Lance（leave from Mozilla）]

Attachment: Verify_Aires KK v2.5.3GP

This bug has been verified as "pass" on the latest build of Flame KK 2.5 and Aries KK 2.5 by the STR in comment 0 .


Actual results: 
1. When the draft has only saved recipient - focus cursor on composer. 
2. When the draft has only saved content - focus cursor on recipient field.
3. When the draft has both recipient & Message content - focus cursor on composer.
See attachment: Verify_Aires KK v2.5.3GP
Reproduce rate: 0/10


Device: Aries KK 2.5 (Pass)
Build ID               20151029010303
Gaia Revision          75f2236d36cc9f9c02d3596fae3de014007cfd82
Gaia Date              2015-10-28 17:02:16
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/1e700005a0ddf2b17803213e1f3f8d78a7a618b8
Gecko Version          44.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151029.002011
Firmware Date          Thu Oct 29 00:20:19 UTC 2015
Bootloader             s1

Device:FlameKK 2.5  (Pass)
Build ID               20151028150209
Gaia Revision          2e89362de40a6c9c36525d36317fa1ae8e67e143
Gaia Date              2015-10-28 04:56:28
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/fc706d376f0658e560a59c3dd520437b18e8c4a4
Gecko Version          44.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20151028.182746
Firmware Date          Wed Oct 28 18:27:57 EDT 2015
Firmware Version       v18D v4 
Bootloader             L1TC000118D0