1207961 - Quick search bar hides behind the VKB when choosing not to display search suggestions

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Teodora Vermesan (:TeoVermesan)]

Attachment: Screenshot_2015-09-24-10-34-03.png

Tested using:
Device: LG G4 (Android 5.1);

Steps to reproduce:
1. Tap something in the URL Bar
2. Choose "No" for "Would you like to turn on search suggestions?"

Expected result:
- no additional suggestions are displayed 
- quick search bar is still displayed above the VKB

Actual result:
- Quick search bar disappears 
- VKB remains displayed
- Tapping outside to dismiss it, quick search bar is displayed on the middle of the screen
- Please take a look at the following video: https://www.youtube.com/watch?v=KQXVOr8a_cU&feature=youtu.be

Note:
- if you open a new tab and type in the URL Bar, quick search bar is not displayed above the VKB on 44 Nightly and 43 Aurora, but is displayed on 42Beta 1 and 41

Comment 1

[Sergej Kravcenko]

Attachment: always reset height

Always reset height, not only on enable.

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8665482 [details] [diff] [review]
always reset height

Review of attachment 8665482 [details] [diff] [review]:
-----------------------------------------------------------------

Nice! Works for me!

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c5f371b2560f

::: mobile/android/base/home/BrowserSearch.java
@@ +729,5 @@
>                          mSuggestionsOptInPrompt = null;
>  
> +                        // Reset the view height
> +                        mView.getLayoutParams().height = LayoutParams.MATCH_PARENT;
> +                        

nit: remove excess whitespace.

Comment 3

[Sergej Kravcenko]

Actually there must be a comment. Two blocks are not related.

Comment 4

[Sergej Kravcenko]

Attachment: always reset height

Added comment

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8667627 [details] [diff] [review]
always reset height

Review of attachment 8667627 [details] [diff] [review]:
-----------------------------------------------------------------

Works for me. Just be careful about adding excessive comments – if the code speaks for itself, there is no need to add a comment. If the code doesn't speak for itself, consider rewriting it to do so. :) It's a little harder in this case because of the pre-existing code.

Comment 6

[Sergej Kravcenko]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8bcecc96505c

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/0d1baecb503a

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0d1baecb503a

Comment 9

[Flaviu Cos]

Verified as fixed in build 44.0a1 2015-10-09;
Device: Motorola Razr (Android 4.4.4).

Comment 10

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Sergej, can you flag this for uplift to 43 (aurora)? You can click details next to the uploaded patch, add approval-aurora = ? (or whatever the name is), and complete in the auto-filled comment.

Comment 11

[Sergej Kravcenko]

Do I need to fill all info fields (Risks, Bug numbers and etc)? I don't know what to write there.

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Sergej Kravcenko from comment #11)
> Do I need to fill all info fields (Risks, Bug numbers and etc)? I don't know
> what to write there.

Add a comment on the fields you're not sure how to fill in, NI me, and I'll fill those in.

Comment 13

[Sergej Kravcenko]

Comment on attachment 8667627 [details] [diff] [review]
always reset height

Approval Request Comment
[Feature/regressing bug #]: 1207961
[User impact if declined]: Quick search bar will not be visible when user decline search suggestions
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: 
[String/UUID change made/needed]:

Comment 14

[Liz Henry (:lizzard)]

Tracking since this looks like a regression from the search suggestions feature.  
42 should be unaffected since search suggestions is aimed for 43.

Comment 15

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Sergej Kravcenko from comment #13)
> [Describe test coverage new/current, TreeHerder]: Sergej & I tested locally, no tests
> [Risks and why]: Low, we choose to do a working piece of code all the time, rather than just under a small subset of conditions. We branch to avoid doing excess work so it'd be fine to do it all the time.

> [String/UUID change made/needed]: None

Comment 16

[Liz Henry (:lizzard)]

Comment on attachment 8667627 [details] [diff] [review]
always reset height

Low risk fix, local testing, ok on m-c. Please uplift to aurora.

Comment 17

[Liz Henry (:lizzard)]

Teo can you verify this on aurora once it lands? Thanks very much!

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c54ea3517760

Comment 19

[Teodora Vermesan (:TeoVermesan)]

After choosing "No" for "Would you like to turn on search suggestions?" the quick search bar is still displayed above the VKB, so:
Verified as fixed using:
Device: Nexus 6 (Android 5.1.1)
Build: Firefox for Android 43.0a2 (2015-10-14)

Comment 20

[Teodora Vermesan (:TeoVermesan)]

This is also affected on Firefox for Android 42 Beta 8