|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0 Build ID: 20161213225041 Steps to reproduce: - Prepare mitm to monitor traffic - Set search.suggest.enabled = false - Write something in the search bar Actual results: - No suggestions being shown - Suggestions being queried Expected results: - No suggestions being shown - No queries.
Reviewing the source code you can see that in SearchEngineRow.updateSuggestions (https://hg.mozilla.org/mozilla-central/file/tip/mobile/android/base/java/org/mozilla/gecko/home/SearchEngineRow.java#l384) the searchSuggestionsEnabled is checked after calling searchEngine.getSuggestions() (https://hg.mozilla.org/mozilla-central/file/tip/mobile/android/base/java/org/mozilla/gecko/home/SearchEngineRow.java#l411).
Ooops, that shouldn't happen of course. Thanks for figuring this out. [Tracking Requested - why for this release]: We're apparently still querying search engine suggestion even if the user disabled them.
Status: UNCONFIRMED → NEW
tracking-fennec: --- → ?
status-firefox51: --- → wontfix
status-firefox52: --- → affected
status-firefox53: --- → affected
status-firefox54: --- → affected
tracking-firefox52: --- → ?
tracking-firefox53: --- → ?
tracking-firefox54: --- → ?
Component: General → Awesomescreen
Ever confirmed: true
OS: Unspecified → Android
Hardware: Unspecified → All
Argh, wrong bug (and I forgot to assign myself here).
Assignee: nobody → jh+bugzilla
Can you confirm that this build (https://queue.taskcluster.net/v1/task/GEqzACs4TjeFjNx4yeE2Hw/runs/0/artifacts/public/build/target.apk) works as expected?
I installed the nightly build and sadly I'm still able to see queries being made even with search suggestions disabled.
Reading more into the source code I can see that getSuggestions() function is just a wrapper that retrieves the suggestions that have been saved asynchronously (?). Don't have much time now to track where they're being saved. By the way, thank you for the quick response!
Comment on attachment 8841330 [details] Bug 1342718 - Don't query for search engine suggestions if we're not displaying them. Right, so this bit needs changing anyway, but apparently the search suggestion are gathered from somewhere else. @carlos: Thanks for giving this a try and no problem - I'll look into it further.
Attachment #8841330 - Flags: review?(s.kaspari)
Okay, this one should hopefully work: https://queue.taskcluster.net/v1/task/RKxrev8VTzyMRcwjegVvdA/runs/0/artifacts/public/build/target.apk
Yes, now it works as expected by default. However I've noticed that after enabling and disabling there are few queries that get leaked. If you have suggestions enabled and then disable them there are a few queries that get sent regardless. I guess it hsa to do with how often suggestion settings are read.
@carlosmelero Third time lucky, I hope :-) https://queue.taskcluster.net/v1/task/baQvKjSxRjCBKle_E0VSaQ/runs/0/artifacts/public/build/target.apk As long as you use the normal preferences menu instead of about:config, the changed setting should take effect immediately now.
Now works great :) Thank you for your work.
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → WORKSFORME
Hold it ;-) - this'll be marked fixed when the patch actually lands. What you've tested was only a custom try build.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Tracking 53/54 for this regression since we should respect what the user does. Not sure whether this is worth tracking for 52 since we are at the end of the cycle.
tracking-firefox53: ? → +
tracking-firefox54: ? → +
Comment on attachment 8841330 [details] Bug 1342718 - Don't query for search engine suggestions if we're not displaying them. https://reviewboard.mozilla.org/r/115580/#review117772
Attachment #8841330 - Flags: review?(s.kaspari) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/f1b38b06e80c Don't query for search engine suggestions if we're not displaying them. r=sebastian
Comment on attachment 8841330 [details] Bug 1342718 - Don't query for search engine suggestions if we're not displaying them. Approval Request Comment [Feature/Bug causing the regression]: Awesomebar search [User impact if declined]: If the user has deactivated live search suggestions from the search engine but not suggestions based on search history, then any search terms entered will be still transmitted to the selected search engine even though the resulting suggestions won't be shown. Also, the display of local search history results may be incomplete in this case. [Is this code covered by automated tests?]: Partially. [Has the fix been verified in Nightly?]: Verified by reporter on a try build incorporating this patch. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: No. [Why is the change risky/not risky?]: No major changes - we already skip getting this data in private mode or if both live *and* search history suggestions are disabled - this patch skips them individually if *either* live or search history suggestions have been disabled. [String changes made/needed]: none
Comment on attachment 8841330 [details] Bug 1342718 - Don't query for search engine suggestions if we're not displaying them. Too late for 52, sorry.
Attachment #8841330 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
status-firefox52: affected → wontfix
tracking-firefox52: ? → -
Status: NEW → RESOLVED
Last Resolved: a year ago → a year ago
status-firefox54: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
Comment on attachment 8841330 [details] Bug 1342718 - Don't query for search engine suggestions if we're not displaying them. Better behavior for search privacy and perf, let's take this in aurora 53.
Attachment #8841330 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
status-firefox53: affected → fixed
You need to log in before you can comment on or make changes to this bug.