Closed Bug 1269481 Opened 8 years ago Closed 8 years ago

Switching search engines doesn't switch suggestions until you restart

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set
normal

Tracking

(firefox47 verified, firefox48 verified, firefox49 verified)

VERIFIED FIXED
Firefox 49
Tracking Status
firefox47 --- verified
firefox48 --- verified
firefox49 --- verified

People

(Reporter: mkaply, Assigned: ahunt)

References

Details

Attachments

(2 files)

If you have two engines with search suggestions and you change to the second one after using the first one, your search suggestions are still based on the first one.

To recreate.

1. Type in tesla. Turn on search suggestions.

Notice your search results (mine were tesla, tesla model 3, tesla model s, tesla stock).

You can go to Yahoo.com and verify these are the first 4 results from autocomplete.

2. Go to settings and change your default to bing.

3. Go back to the search bar and search on tesla again. You will get the same results you got from yahoo. Go to bing.com and you will see:

tesla, tesla model 3, tesla motors, tesla 3

If you restart the browser (as in kill it and restart), you will see the correct search suggestions.

So Fennec appears to be caching the search suggestions URL and not updating it when the default search engine is changed.
Blocks: 851969
I think this is the right fix.

Just remove the suggest client if the search engine changes...
Attachment #8747972 - Flags: review?(michael.l.comella)
Attachment #8747972 - Flags: review?(michael.l.comella) → review?(ahunt)
Comment on attachment 8747972 [details] [diff] [review]
Null out suggest client when search engine changes

This doesn't seem to work for me. The issue is that we update the client in ensureSuggestClientIsSet, which is performed earlier.

I've prepared an alternative patch which seems to fix the issue for me (sorry, this code is pretty messy, we should probably clean it up at some point...).
Attachment #8747972 - Flags: review?(ahunt)
Comment on attachment 8748778 [details]
MozReview Request: Bug 1269481 - Update suggestClient if suggestTemplate changes r?mkaply

That's funny. Your fix was what I was going to do originally but I couldn't come up with the right way to do it.

Definitely a better idea.
Attachment #8748778 - Flags: review?(mozilla) → review+
https://hg.mozilla.org/integration/fx-team/rev/48fd2d23e181161cb97ec2b91049fc09200439bc
Bug 1269481 - Update suggestClient if suggestTemplate changes r=mkaply
This is pretty bad. Have we figured out whether this is a regression? Can we uplift this?
Assignee: nobody → ahunt
Flags: needinfo?(mozilla)
Flags: needinfo?(ahunt)
(In reply to :Margaret Leibovic from comment #6)
> This is pretty bad. Have we figured out whether this is a regression? Can we
> uplift this?

This is probably a regression from 2013:
http://hg.mozilla.org/mozilla-central/rev/1a6e28e051e0

The patch is quite simple, so I'll request uplift to aurora+beta.
Flags: needinfo?(ahunt)
Comment on attachment 8748778 [details]
MozReview Request: Bug 1269481 - Update suggestClient if suggestTemplate changes r?mkaply

Approval Request Comment
[Feature/regressing bug #]: Bug 896560
[User impact if declined]: Search suggestions are fetched from old search engine after switching default search engine, until app is restarted.
[Describe test coverage new/current, TreeHerder]: Manual testing to ensure search suggestions change when search engine is switched.
[Risks and why]: Low risk, we add an additional condition to avoid an early return when the search engine has switched.
[String/UUID change made/needed]: None.
Attachment #8748778 - Flags: approval-mozilla-beta?
Attachment #8748778 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/48fd2d23e181
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Comment on attachment 8748778 [details]
MozReview Request: Bug 1269481 - Update suggestClient if suggestTemplate changes r?mkaply

This a pretty code scenario around search engine defaults and changing them, Aurora48+, Beta47+
Attachment #8748778 - Flags: approval-mozilla-beta?
Attachment #8748778 - Flags: approval-mozilla-beta+
Attachment #8748778 - Flags: approval-mozilla-aurora?
Attachment #8748778 - Flags: approval-mozilla-aurora+
Hello Ioana, could you please do some focused testing on this for 47.0b4? Thanks!
Flags: needinfo?(chiorean.ioana)
Flags: needinfo?(mozilla)
(In reply to Ritu Kothari (:ritu) from comment #11)
> Hello Ioana, could you please do some focused testing on this for 47.0b4?
> Thanks!

Yes! I am adding it to the signoff testplan
Flags: needinfo?(chiorean.ioana) → needinfo?(ioana.chiorean)
Verified as fixed on Firefox 47 Beta 4 on Samsung Galaxy S6 Edge (Android 5.1.1)
Verified as fixed on latest Nightly and Aurora on Samsung Galaxy S6 Edge (Android 5.1.1)
Status: RESOLVED → VERIFIED
Thanks Mihai!
Flags: needinfo?(ioana.chiorean)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.