904640 - [Messages] Keyboard should always close when pressing Send

Status: RESOLVED FIXED

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

Description

[Rick Waldron [:rwaldron]]

copied from https://bugzilla.mozilla.org/show_bug.cgi?id=903947#c4



1.Add 2~3 contacts-> Enter text in message field->Drag down the 'to field'->Send the message
Actual: Once the send button is pressed,the Keyboard is still shown in the thread_list
Expected: The keyboard should disapper once the send button is pressed.

Requesting leo+ because the other was marked as such

Comment 1

[Rick Waldron [:rwaldron]]

Attachment: Github

Comment 2

[Rick Waldron [:rwaldron]]

Attachment: Github Pull Request https://github.com/mozilla-b2g/gaia/pull/11510

- Calls this.sendButton.focus() inside ThreadUI.onSendClick
- Test by checking spy intel for focus.called

Comment 3

[Julien Wajsberg [:julienw]]

Comment on attachment 789655 [details] [review]
Github Pull Request https://github.com/mozilla-b2g/gaia/pull/11510

so I'm not so against this solution, but I proposed another one that feels better to me. We can still argue though :)

Comment 4

[Rick Waldron [:rwaldron]]

Updated change outline

- Remove focus from all elements before panel change by calling document.activeElement.blur() in MessageManager.onHashChange
- Updates Mocks: 
    - MockThreadListUI to contain all properties (data and method)
    - MockThreadUI to contain all properties (data and method)
- Tests MessageManager.onHashChange:
    - Remove any focus left on specific elements
        - Assert on spy intel for document.activeElement.blur
    - Exit edit mode (Thread or Message) 
        - Assert on spy intel for:
            - ThreadUI.cancelEdit
            - ThreadListUI.cancelEdit
    - Reset Group Participants View 
        - Assert on spy intel for ThreadUI.groupView.reset

Comment 5

[Julien Wajsberg [:julienw]]

Comment on attachment 789655 [details] [review]
Github Pull Request https://github.com/mozilla-b2g/gaia/pull/11510

r=me with nits

as usual, check that the test passes before checking it in ;)

thanks !

Comment 6

[Rick Waldron [:rwaldron]]

gaia@master https://github.com/mozilla-b2g/gaia/commit/1a38ad4df2edb9b608f77f1691cb6c4450f98d8e

gaia@v1-train https://github.com/mozilla-b2g/gaia/commit/dd3959fa74e356a528daa76ffee14c2c728a4b56

Comment 7

[Leo]

Hello Rick and Julienw,
This issue is still reproducible.

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

v1.1.0hd: dd3959fa74e356a528daa76ffee14c2c728a4b56

Comment 9

[Leo]

Issue is still reproducible.
STR:
1.Add 2~3 contacts -> Enter some text in the message field -> Drag down the 'To field' -> send the message.
The screen is moved to thread_list but the keyboard doesn't disappear.

Comment 10

[Julien Wajsberg [:julienw]]

Are you sure you use a build that contains that fix ?

Because your STR is exactly what I tried when I tested the fix before I gave r+, but I may have missed something.

Comment 11

[Leo]

Hi Julienw,
I have checked the code after testing,this fix is available.

Comment 12

[Julien Wajsberg [:julienw]]

Sorry this is not reproducible for me.

Rick> you should not have merged this to v1-train as this is not leo+...

Triagers> I'm very sorry this landed on v1-train without leo. This is a no-risk patch (no change in logic, all this is well tested) asked by leo but we can revert it if you think this is an error.

Comment 13

[Rick Waldron [:rwaldron]]

(In reply to Julien Wajsberg [:julienw] from comment #12)
> Sorry this is not reproducible for me.
> 
> Rick> you should not have merged this to v1-train as this is not leo+...

You're right and I apologize for any confusion or time wasted by this mistake. I had assumed that because the ticket that spawned this (bug 903947) was leo+ that this too would be leo+.

Comment 14

[Rick Waldron [:rwaldron]]

Updating that I was responsible for this

Comment 15

[John Ford [:jhford] CET/CEST Berlin Time]

I was not able to uplift this bug to v1-train.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1-train, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1-train
  git cherry-pick -x  1a38ad4df2edb9b608f77f1691cb6c4450f98d8e
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 16

[Rick Waldron [:rwaldron]]

John, 

Apologies for the confusion, I have already merged this into v1-train. I had assumed that because the bug that spawned this bug was leo+, that this would implicitly be leo+ and prematurely uplifted to v1-train

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

Again, my apologies for the confusion and time wasted addressing this issue.

Comment 17

[Julien Wajsberg [:julienw]]

Rick> you really forgot to update the "status-b2g18" flag when you uplifted the patch :)

Comment 18

[Rick Waldron [:rwaldron]]

(In reply to Julien Wajsberg [:julienw] from comment #17)
> Rick> you really forgot to update the "status-b2g18" flag when you uplifted
> the patch :)

\o/

Comment 19

[sbogati]

Issue No longer repros. on Leo device 
Build ID: 20130815041201
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/692d3414bb12
Gaia: 0f1f1ab0ab31a1df8a780baa048b5e7b2854205d
Platform Version: 18.1
 After text sent the keyboard went away automatically.

Comment 20

[Julien Wajsberg [:julienw]]

Leo> is it fixed for you too ?

Comment 21

[Leo]

Hi Julienw,
We are still able to reproduce the issue.
The following error is shown when checking this scenario

08-17 09:11:49.199: E/GeckoConsole(508): [JavaScript Error: "TypeError: last is null" {file: "app://sms.gaiamobile.org/js/recipients.js" line: 608}]

Thanks,

Comment 22

[Leo]

The last value is shown as null in the following line while (last.isPlaceholder) {

Comment 23

[Julien Wajsberg [:julienw]]

This error can happen only if we don't have any recipient or placeholder in the recipient panel. Rick, is this a possible case ?

And even if this happens, this should not prevent the keyboard from hiding, because this happens in an unrelated piece of code.

Leo, can you share your detailed STR (Steps To Reproduce) with us ? and also your gaia commit hash ? Is it possible that you don't have the same code as our Gaia v1-train branch ?

Comment 24

[Leo]

Hi Julienw,

We have tested the scenario with latest b2g-18(gecko) and latest v1-train,the issue is reproducible.
Need to test multiple times (Tested around 10 times).
STR:
1. Add 2~3contacts from contacts application
2. Add text
3. Focus should be either in 'to' or 'message(text)' field.
4. Drag down the 'to field' -> send message

The keyboard is not disappearing.

Thanks,

Comment 25

[Julien Wajsberg [:julienw]]

Leo> if this happens only once every 10 times, I'd say it's not the same bug, please file a new bug then. Plus I don't think it should block the leo release.

Comment 26

[Rick Waldron [:rwaldron]]

(In reply to Leo from comment #21)
> Hi Julienw,
> We are still able to reproduce the issue.
> The following error is shown when checking this scenario
> 
> 08-17 09:11:49.199: E/GeckoConsole(508): [JavaScript Error: "TypeError: last
> is null" {file: "app://sms.gaiamobile.org/js/recipients.js" line: 608}]

This was being caused by the call to ThreadUI.recipients.visible(...) in #thread=n view because the event handlers for ThreadUI.assimilateRecipients still exist in that view. Dragging the To field removes the placeholder at the end of the recipients list. 

This was resolved in https://bugzilla.mozilla.org/show_bug.cgi?id=905950, see https://bugzilla.mozilla.org/show_bug.cgi?id=905950#c10 for change summary

Comment 27

[Julien Wajsberg [:julienw]]

Rick, the error message is not exactly the same though, are you sure this is fixed in bug 905950 ?

Comment 28

[Rick Waldron [:rwaldron]]

(In reply to Julien Wajsberg [:julienw] from comment #27)
> Rick, the error message is not exactly the same though, are you sure this is
> fixed in bug 905950 ?

No, this is actually different—thanks for leaning on this one. I'll follow up shortly.

Comment 29

[Rick Waldron [:rwaldron]]

https://bugzilla.mozilla.org/show_bug.cgi?id=906377