1179731 - Autocomplete suggestions pop up can be seen in awesomescreen/url suggestions

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Ioana Chiorean]

Attachment: Screenshot_2015-07-02-14-26-25.png

Fennec 41.0a2, 42.0a1
Nexus 7  (Android 5.1)

Steps:
1. Go to bugzilla.mozilla.org perform a search with more than 3 different terms
2. Tap the search to have the Autocomplete suggestions open
3. Tap URL bar and add some address/characters

Expected Results:
- the awesome screen should be filler with suggestions related to the characters entered
- no graphic issue should be seen

Actual Results:
- the Autocomplete suggestions pop up from previous form can be seen under the url suggestions 

Note:
- see attached screenshot
- might be a regression from Bug 1137483 - Add a "quick search" bar of search engines

Comment 1

[Teodora Vermesan (:TeoVermesan)]

regression window:
not affected build: 20-05
affected build: 21-05

pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ac277e615f8f&tochange=b9424d63fe35

Bug 1137483 - Add a "quick search" bar of search engines?

Comment 2

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

We removed the background on BrowserSearch [1] in the regressing bug in favor of setting the window background. Presumably, we were drawing over the auto-complete suggestions before (who let that overdraw in!).

Best solution would be to hide the auto-complete when opening the home screen. Investigating...

[1]: https://reviewboard.mozilla.org/r/7551/diff/3#3

Comment 3

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

(In reply to Michael Comella (:mcomella) from comment #2)
> Best solution would be to hide the auto-complete when opening the home
> screen. Investigating...

Seemed easy enough but FormAssistPopup.onMetricsChanged gets called before I call hide. onMetricsChanged queues a runnable to positionAndShowPopup, which runs after hide and undoes the effect of it. x_x

I think I can move hide before onMetricsChanged...

Comment 4

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

Attachment: MozReview Request: Bug 1179731 - Hide autocomplete suggestions when showing the home pager. r=sebastian

Bug 1179731 - Hide autocomplete suggestions when showing the home pager. r=sebastian

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8631357 [details]
MozReview Request: Bug 1179731 - Hide autocomplete suggestions when showing the home pager. r=sebastian

https://reviewboard.mozilla.org/r/12899/#review11493

Ship It!

Comment 6

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

url:        https://hg.mozilla.org/integration/fx-team/rev/15700237422088a40530b7d2f5603019cfb2e675
changeset:  15700237422088a40530b7d2f5603019cfb2e675
user:       Michael Comella <[e-mail]>
date:       Wed Jul 08 17:53:25 2015 -0700
description:
Bug 1179731 - Hide autocomplete suggestions when showing the home pager. r=sebastian

Comment 7

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

https://hg.mozilla.org/mozilla-central/rev/157002374220

Comment 8

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

Attachment: (41 patch) Add null check to autocomplete suggestions

Comment 9

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

(In reply to Michael Comella (:mcomella) from comment #8)
> Created attachment 8632199 [details] [diff] [review]
> (41 patch) Add null check to autocomplete suggestions

Commit description is useful here:

bug 1085591 landed, meaning we no longer have to check if this View is null
because it's guaranteed to get initialized by the time the HomePager is shown.

However, bug 1085591 is not in 41 so we have to add a null check for uplift to
41.

Comment 10

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

Comment on attachment 8631357 [details]
MozReview Request: Bug 1179731 - Hide autocomplete suggestions when showing the home pager. r=sebastian

This request is for this patch and the additional branch patch.

Approval Request Comment
[Feature/regressing bug #]: bug 1137483.
[User impact if declined]: Users will see auto-complete suggestions over the awesomescreen (see screenshot).

[Describe test coverage new/current, TreeHerder]: Tested locally.
[Risks and why]: Low - we simply hide View when we show another one. There was a asynchronicity scare but that was figured out (and mentioned in the comments).

[String/UUID change made/needed]: None

Comment 11

[Kate Glazko]

(In reply to Michael Comella (:mcomella) from comment #9)
> However, bug 1085591 is not in 41 so we have to add a null check for uplift
> to
> 41.

Does this mean that you have to do something extra in this patch prior to uplifting? Or is it safe to uplift now?

Comment 12

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

(In reply to Kate Glazko from comment #11)
> Does this mean that you have to do something extra in this patch prior to
> uplifting? Or is it safe to uplift now?

Both my mozreview (comment 4) and unreviewed null check patch (comment 8) should be uplifted (obviously, pending review :D).

Comment 13

[Kate Glazko]

Thanks for the clarification Michael, awaiting your pending review :)

Comment 14

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8632199 [details] [diff] [review]
(41 patch) Add null check to autocomplete suggestions

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

Ship it! :)

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8631357 [details]
MozReview Request: Bug 1179731 - Hide autocomplete suggestions when showing the home pager. r=sebastian

Both patches seem safe. Let's uplift BOTH to Aurora.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

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

Comment 17

[Flaviu Cos]

Verified as fixed in builds:
- 42.0a1 2015-07-16;
- 41.0a2 2015-07-16;
Device: Asus Transformer Pad (Android 4.2.1).