1230568 - Search suggestions are disabled in normal browsing after going to private browsing without choosing to turn off search suggestions

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Teodora Vermesan (:TeoVermesan)]

Steps to reproduce:
1. Go to normal browsing and start typing in the URL Bar ("br")
("would you like to turn on search suggestions" message is displayed)
2. Don't choose anything and go to private browsing and start typing in the URL Bar ("br"). See that no search suggestions are displayed
3. Go back to normal browsing and start typing in the URL Bar ("br")

Expected results:
- "would you like to turn on search suggestions" message is displayed

Actual results:
- no message is displayed and no search suggestions are displayed
- search suggestions appear to be disabled

Note:
- no reproducible on 42.0.2 and 43 Beta 8
- reproducible using latest Aurora and Nightly

Comment 1

[Prabhjyot Sodhi [:psd]]

Hi!
From a quick look, it seems that we should set `mSuggestionsOptInPrompt` to null at [1], so that the showSuggestionsOptIn ([2]) works as it is supposed to. Alternatively (a better approach) we could try to change the view's state from gone to visible. I won't be tackle this immediately. But maybe this could be a good first bug or nth bug.
If nobody takes it by around the 20th, I'll probably take it up!

[1]: https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/home/BrowserSearch.java#793
[2]: https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/home/BrowserSearch.java#749

Comment 2

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

I think we'll need to fix this before the 20th so I'll take it.

Comment 3

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

Thanks for the thorough analysis, Prabhjyot! :)

Comment 4

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

Spoke with ahunt – he'll actually take it from here. :) If you need some help around the code, let me know.

Comment 5

[Andrzej Hunt :ahunt]

Attachment: MozReview Request: Bug 1230568 - Allowing showing search suggestion opt in multiple times r=mcomella

Bug 1230568 - Allowing showing search suggestion opt in multiple times r=mcomella

Comment 6

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

Comment on attachment 8696151 [details]
MozReview Request: Bug 1230568 - Allowing showing search suggestion opt in multiple times r=mcomella

https://reviewboard.mozilla.org/r/27285/#review24843

Nice!

::: mobile/android/base/home/BrowserSearch.java:747
(Diff revision 1)
> -        // Return if the ViewStub was already inflated - an inflated ViewStub is removed from the
> +        // Simply make the ViewStub visible again if it has already previously been shown.

mega-nit: Simply -> only

Only means do this and nothing else but simply doesn't have the same implication.

Comment 7

[Andrzej Hunt :ahunt]

Comment on attachment 8696151 [details]
MozReview Request: Bug 1230568 - Allowing showing search suggestion opt in multiple times r=mcomella

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27285/diff/1-2/

Comment 8

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

Michael, please request uplift to Beta44 (when ready) as this also impacts FF44. Thanks!

Comment 9

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

This is a key scenario, tracked for FF44.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/97cbdba9513d

Comment 11

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

Andrzej, can you request uplift to 44?

Comment 12

[Andrzej Hunt :ahunt]

Comment on attachment 8696151 [details]
MozReview Request: Bug 1230568 - Allowing showing search suggestion opt in multiple times r=mcomella

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: Users may not discover the ability to enable search suggestions if the prompt is overseen when first shown - enabling search suggestions in that case requires opening the application settings.
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: low risk - this changes visibility of a view that already exists.
[String/UUID change made/needed]: none

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/97cbdba9513d

Comment 14

[Andrzej Hunt :ahunt]

Comment on attachment 8696151 [details]
MozReview Request: Bug 1230568 - Allowing showing search suggestion opt in multiple times r=mcomella

Adding approval request for beta too. Sorry, I got confused by the version changes yesterday, and erroneously thought 44 was still aurora, whereas it's now beta.

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: Users may not discover the ability to enable search suggestions if the prompt is overseen when first shown - enabling search suggestions in that case requires opening the application settings.
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: low risk - this changes visibility of a view that already exists.
[String/UUID change made/needed]: none

Comment 15

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

Teodora, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!

Comment 16

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

Comment on attachment 8696151 [details]
MozReview Request: Bug 1230568 - Allowing showing search suggestion opt in multiple times r=mcomella

Key scenarios that needs to be fixed soon, Beta44+, Aurora45+

Comment 17

[Teodora Vermesan (:TeoVermesan)]

After returning from private to normal browsing and start typing something in the URL Bar, "Would you like to turn on search suggestions" message is displayed, so:
Verified as fixed using:
Device: Samsung Galaxy Tab S2 (Android 5.0.2)
Build: Firefox for Android 46.0a1 (2015-12-17)

Comment 18

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

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

Comment 19

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

This is hitting conflicts uplifting it to beta, likely due to the lack of bug 1107811. Can we get a rebased patch in here to work around it, if this really needs to go to beta?

Comment 20

[Teodora Vermesan (:TeoVermesan)]

After returning from private to normal browsing and start typing something in the URL Bar, "Would you like to turn on search suggestions" message is displayed, so:
Verified as fixed using:
Device: Nexus 6 (Android 6.0)
Build: Firefox for Android 45.0a2 (2015-12-23)

Comment 21

[Andrzej Hunt :ahunt]

Attachment: 1230568_backport_beta.patch

Here's the rebased patch!

Comment 22

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

https://hg.mozilla.org/releases/mozilla-beta/rev/19b9d400cf8e

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/19b9d400cf8e