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)
Firefox for Android Graveyard
General
Tracking
(firefox47 verified, firefox48 verified, firefox49 verified)
VERIFIED
FIXED
Firefox 49
People
(Reporter: mkaply, Assigned: ahunt)
References
Details
Attachments
(2 files)
1.25 KB,
patch
|
Details | Diff | Splinter Review | |
58 bytes,
text/x-review-board-request
|
mkaply
:
review+
ritu
:
approval-mozilla-aurora+
ritu
:
approval-mozilla-beta+
|
Details |
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.
Reporter | ||
Comment 1•8 years ago
|
||
I think this is the right fix. Just remove the suggest client if the search engine changes...
Attachment #8747972 -
Flags: review?(michael.l.comella)
Reporter | ||
Updated•8 years ago
|
Attachment #8747972 -
Flags: review?(michael.l.comella) → review?(ahunt)
Assignee | ||
Comment 2•8 years ago
|
||
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)
Assignee | ||
Comment 3•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/50529/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/50529/
Attachment #8748778 -
Flags: review?(mozilla)
Reporter | ||
Comment 4•8 years ago
|
||
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+
Assignee | ||
Comment 5•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/48fd2d23e181161cb97ec2b91049fc09200439bc Bug 1269481 - Update suggestClient if suggestTemplate changes r=mkaply
Comment 6•8 years ago
|
||
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)
Assignee | ||
Comment 7•8 years ago
|
||
(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)
Assignee | ||
Comment 8•8 years ago
|
||
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?
Comment 9•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/48fd2d23e181
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
status-firefox47:
--- → affected
status-firefox48:
--- → affected
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)
Comment 12•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/8aa028f3da50
Comment 13•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/655bc3d29ea2
Reporter | ||
Updated•8 years ago
|
Flags: needinfo?(mozilla)
Comment 14•8 years ago
|
||
(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)
Comment 15•8 years ago
|
||
Verified as fixed on Firefox 47 Beta 4 on Samsung Galaxy S6 Edge (Android 5.1.1)
Comment 16•8 years ago
|
||
Verified as fixed on latest Nightly and Aurora on Samsung Galaxy S6 Edge (Android 5.1.1)
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•