1070895 - Pressing ENTER in search box clears search and returns back to inbox

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[Jan Jongboom [:janjongboom] (inactive)]

1. Open the e-mail client
2. Scroll up
3. Type search term
4. Press ENTER to exec search term and dismiss keyboard
5. Back in inbox again
6. &*#$&#*$&(

On Flame 2.0 & 2.2

Comment 1

[Jan Jongboom [:janjongboom] (inactive)]

Attachment: Patch

So this is what happens:

- The UI element for 'Cancel' is in the wrong place. It should not be top corner because none of our back/cancel elements are there. Because you need to style it there type="submit" is added on here.
- When pressing enter we submit the form, therefore the onCancelSearch function is called, which starts playing animation sometimes
- The form does not receive a preventDefault() from cancelSearch and has no action so it submits to itself, refreshing the app
- Last shown inbox is shown

I fixed it so the UI stays the same, but we should move the Cancel button to the left as well.

Comment 2

[Jan Jongboom [:janjongboom] (inactive)]

[Blocking Requested - why for this release]: Searching is almost impossible in the email client. Keyboard does not dismiss, and pressing Enter clears all state in the app.

Comment 3

[James Burke [:jrburke]]

Comment on attachment 8493091 [details] [review]
Patch

Great find, thanks for doing a patch!

By fixing this, it uncovers an issue with autocorrect, and I would like to see a baseline "submit" listener in place just so we can properly cancel form submission if the submit pathways change in the future.

I will attach a new pull request since I ended up doing the other changes in the progress of reviewing this patch. So I will obsolete this patch, but only because the existing email code was deficient in other ways that were only found once the fix in this patch fixed the form preventDefault.

Comment 4

[James Burke [:jrburke]]

Attachment: GitHub pull request

This issue was caused by the changes in bug 1001298, and as noted in :janjongboom's excellent investigation into the issue, the type="submit" on the Cancel form is just semantically incorrect. 

I filed bug 1071408 against the input_areas.css as this is a shared style and layout for a search area, also used in Contacts, and it is best for it be fixed there long term. 

If the Cancel button should be on the different side of the search field, that fix would also make sense as part of bug 1071408, but I leave it to the building block owners to determine that.

However, even with a fix to the type="submit" on the Cancel, we should still do the changes in this pull request:

* Makes sure a 'submit' listener is in place to stop accidental form submissions (usually by the Enter button on keyboard) from trying to submit the form. This will give us protection if the building block moves away from type="submit" or some other submit pathway shows up later.

* For the click handler for Cancel, make sure it is a real Cancel click and not a simulated one from a submit action.

* Turn off autocorrect for the search input. Now that the Enter button does not destroy the card, the text being autocorrected by the keyboard on Enter.

So, this will give is a good path forward if that form structure changes, and if this is approved for 2.1, allows us to work with the weird type="submit" in that branch.

Tested on flame device.

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8493529 [details] [review]
GitHub pull request

Thanks very much for making the effort to both file the bug and provide a fix, Jan!  I know how frustrating these things can be and it's awesome that you are able to help address these problems directly!  Maybe the reason I don't recognize the curse word pattern in STR6 is that it's a very restrained curse word! ;)

Thanks very much James for the expanded comments and investigation plus spin-offs.  minor nit on also including "inputmode" in addition to x-inputmode.

Comment 6

[James Burke [:jrburke]]

Merge from master:
https://github.com/mozilla-b2g/gaia/commit/9b6a40245221138dac945e1c1724742c5ce16d6c

from pull request:
https://github.com/mozilla-b2g/gaia/pull/24321

Includes inputmode suggestion from review.

Comment 7

[James Burke [:jrburke]]

This change is likely easy to uplift to 2.0, and is low risk. I may need to do a special pull request for it, but there is a good change it will just auto-uplift via a cherrypick.

As to the severity of the issue: search is usable, as long as the user does not tap the Enter button on the keyboard. The search results view is just a bit short since the keyboard is up. That search area is scrollable though with the keyboard up.

It will be surprising to the user if they tap Enter though. One of the reasons I did not catch this before because I just do not press enter when I have used search, but rather did the search and refined it until I saw the email I was looking for or scrolled then tapped, without closing the keyboard via Enter.

To be clear, I would prefer to have this fixed on the branches for robustness, and I am happy to turn around any special changesets to get it uplifted to 2.0, 2.1, if auto uplift does not work. Just giving more context as to the scope of the issue. If for some reason 2.0 is not approved, this should definitely go to 2.1.

Comment 8

[Dylan Oliver [:doliver]]

triage: Because the search function does work (just maybe not quite how you expect it to), this one does not fit the criteria for blocking, especially on 2.0 at this point. Let's request approval for 2.1 uplift.

Comment 9

[James Burke [:jrburke]]

Comment on attachment 8493529 [details] [review]
GitHub pull request

[Bug caused by] (feature/regressing bug #):
Bug 1001298

[User impact] if declined:
Pressing Enter on the search form destroys the search card and returns the user to the message list.

[Testing completed]:
Tested on flame device.

[Risk to taking this patch] (and alternatives if risky):
Very low. As the form event handling is more robust, it decreases risk in the future if the submit pathways are changed.

[String changes made]:
none

Comment 10

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/012107b740702951bfd2414faf7b396ca3bacb63

Comment 11

[Edward Chen[:edchen]]

[Environment]
Gaia-Rev        13973ab50760d1e8bb773082163f0dff19d35a44
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-aurora/rev/6e317e075d04
Build-ID        20140928160204
Version         34.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20140925.192608
FW-Date         Thu Sep 25 19:26:18 EDT 2014
Bootloader      L1TC10011800

[Result]
PASS