925307 - [Contacts] In search mode, clicking ENTER on the keyboard exits the search mode

Status: RESOLVED FIXED

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

Description

[Germán Toro del Valle (:gtorodelvalle)]

TO REPRODUCE:
1. Open the Contacts app.
2. Click on the search bar. The search mode opens.
3. Click on the INTRO key on the keyboard.

EXPECTED RESULT:
4. The search mode is not exited.

OBSERVED RESULT:
4. The search mode is exited and the Contacts app goes back to normal mode.

Comment 1

[Germán Toro del Valle (:gtorodelvalle)]

Attachment: 12771.html

Comment 2

[Jose Manuel Cantera]

https://github.com/mozilla-b2g/gaia/commit/7f240f2da3178b275315790585396629dbe2d756

Comment 3

[Joe Cheng [:jcheng] (please needinfo)]

ni? ayman to confirm the spec. this should be working as intended. may need to back out after Ayman confirms

Comment 4

[Germán Toro del Valle (:gtorodelvalle)]

In fact, this was the way it was implemented. The inclusion of the new HTML lazy loading functionality just make this stop functioning so somehow we introduced a regression this patch fixes ;-)

Comment 5

[Preeti Raghunath(:Preeti)]

Hi,

What exactly is the intro key?

How does it impact the user if not fixed?

Comment 6

[Germán Toro del Valle (:gtorodelvalle)]

Hey! My bad, I meant the ENTER key (http://en.wikipedia.org/wiki/Enter_key). Regarding the user, the problem is that clicking on this key the search mode exits and the user comes back to the normal mode. The user may be tempted to click this return key until she notices that the search is made automatically as she types characters :-)

Comment 7

[isabelrios]

Attachment: 2013-10-18-04-45-15.png

Comment 8

[isabelrios]

Plase see screenshot attached.
Besides the Search mode is exited, the screen is not correctly shown.

Comment 9

[Germán Toro del Valle (:gtorodelvalle)]

Just tested it with Gecko-c4665b7 and latest Gaia from master (as for some minutes ago) and it is working fine ;-)

Comment 10

[Germán Toro del Valle (:gtorodelvalle)]

Waiting for Ayaman's feedback to request approval for v1.2 ;-)

Comment 11

[ayman maat :maat]

(In reply to Joe Cheng [:jcheng] from comment #3)
> ni? ayman to confirm the spec. this should be working as intended. may need
> to back out after Ayman confirms

Hey guys. As far as i am aware tapping on the INTRO (ENTER) key on the keyboard when the contacts list is in search mode and there is nothing in the search text input field should result in nothing happening. search mode should not be existed and the keyboard should not be closed... nothing should happen. The user should remain in search mode. The screenshot in comment 7 demonstrates incorrect behaviour.

We have a dragdown action on the keyboard implemented that closes it.

Comment 12

[Preeti Raghunath(:Preeti)]

(In reply to gtorodelvalle from comment #9)
> Just tested it with Gecko-c4665b7 and latest Gaia from master (as for some
> minutes ago) and it is working fine ;-)

So this issue is not reproducible?

Hi Ayman maat :maat
What you are suggesting seems is that there is a fix for the dragdown action on keyboard? Is that an enhancement to 1.2?

Comment 13

[ayman maat :maat]

(In reply to Preeti Raghunath(:Preeti) from comment #12)
> (In reply to gtorodelvalle from comment #9)
> > Just tested it with Gecko-c4665b7 and latest Gaia from master (as for some
> > minutes ago) and it is working fine ;-)
> 
> So this issue is not reproducible?
> 
> Hi Ayman maat :maat
> What you are suggesting seems is that there is a fix for the dragdown action
> on keyboard? Is that an enhancement to 1.2?

nop, i am not suggesting that. I was just observing that the is now the ability to drag down keyboard. I do not know if this is an enhancement to V1.2 because i have not been part of the team that introduced that action.

Comment 14

[Joe Cheng [:jcheng] (please needinfo)]

triage: should not block release. please ask for approval to land in v1.2. tks

Comment 15

[Germán Toro del Valle (:gtorodelvalle)]

Attachment: 13069.html

My great contribution to Science for today :-)

As suggested by Ayman, clicking on the ENTER key when in Search mode and the focus in on the search input text field does nothing: (1) it does NOT close the search mode and (2) it does NOT collapse the keyboard.

As soon as I have José Manuel's r+, we'll merge this new patch and ask for v1.2 approval.

Thanks!

Comment 16

[Germán Toro del Valle (:gtorodelvalle)]

Since we were hiding the keyboard when clicking on the ENTER key and this was not the desired behavior ;-)

Comment 17

[Jose Manuel Cantera]

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

Comment 18

[Germán Toro del Valle (:gtorodelvalle)]

Comment on attachment 815331 [details]
12771.html

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 #): New lazy HTML loading mechanism
[User impact] if declined: Bad experience (leaving the Search mode probably unexpectedly)
[Testing completed]: On-device testing
[Risk to taking this patch] (and alternatives if risky): Very low
[String changes made]: None

Comment 19

[Germán Toro del Valle (:gtorodelvalle)]

Comment on attachment 821704 [details]
13069.html

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 #): New lazy HTML loading mechanism
[User impact] if declined: Bad experience (leaving the Search mode probably unexpectedly)
[Testing completed]: On-device testing
[Risk to taking this patch] (and alternatives if risky): Very low
[String changes made]: None

Comment 20

[Germán Toro del Valle (:gtorodelvalle)]

PLEASE NOTICE THAT THIS PATCH IS COMPOSED OF 2 PULL REQUESTS AND CONSEQUENTLY OF 2 COMMITS ;-) Thanks!

Comment 21

[isabelrios]

Working correctly on master (10/29 build):
Gecko-5fff789
Gaia-9fa7a86

Comment 22

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 815331 [details]
12771.html

moving code to the correct place, no risky.

a=me

Comment 23

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 821704 [details]
13069.html

Follow up of the previous bug, also non risky

a=me

Comment 24

[Noemí Freire (:noemi)]

Hi John,

Could you please help us with the uplift to v1.2? thanks!

Comment 25

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

There's a merge conflict:

#	both modified:      apps/communications/contacts/js/views/search.js

Comment 26

[Germán Toro del Valle (:gtorodelvalle)]

Hi John, I guess the problem was due to the fact that this bug included a couple of pull requests and consequently a couple of commits. I just ran:

git checkout v1.2
git cherry-pick -x 0e085759b69f24ef9c8d1299ed1bd4d7a3268528
git cherry-pick -x bac2948c4ee6aecdd5d622f47866b9631acbd762
git push remote-mozilla-b2g-gaia v1.2

without problems.

So I just merged them in v1.2:
https://github.com/gtorodelvalle/gaia/commit/0e085759b69f24ef9c8d1299ed1bd4d7a3268528 -> https://github.com/gtorodelvalle/gaia/commit/bf77957071a61f3938d09e3147f88bf0a315a9f1
https://github.com/gtorodelvalle/gaia/commit/bac2948c4ee6aecdd5d622f47866b9631acbd762 -> https://github.com/gtorodelvalle/gaia/commit/d4d1709a7fdd376aa9390ad863e41569cbc59f91