Closed Bug 1182792 Opened 4 years ago Closed 4 years ago

Search Suggestions should be disabled in Private Browsing Mode

Categories

(Firefox :: Search, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 42
Iteration:
42.2 - Jul 27
Tracking Status
firefox42 --- verified

People

(Reporter: dholbert, Assigned: adw)

References

Details

(Whiteboard: [suggestions][fxsearch])

Attachments

(1 file)

STR:
 1. Open a "private browsing" window in Firefox Nightly (ctrl shift p)
 2. Type a word into the URL bar.

ACTUAL RESULTS: Search suggestions appear as I type. (So, my "private" browsing URL-entries are being shared with my search provider.)

EXPECTED RESULTS: Search suggestions should be disabled in Private Browsing Mode.


For comparison, I tested Firefox Nightly for Android & Chrome Dev channel on my linux laptop.  Both appear to disable their search-suggestions feature in their Private Browsing modes.
(Note: yes, private browsing is primarily about not storing traces *locally* on the machine, and search suggestions are orthogonal to that. Nonetheless: users who've activated private browsing mode are unlikely to want their URL-bar keystrokes sent to any 3rd-party service, and we should have their back.)
mak, looks like you've been working on bugs in this area; maybe you can take this, or suggest someone who can?
Flags: needinfo?(mak77)
There are various things to evaluate here:

1. Until we still have the search bar, it's likely we can provide the best "private" experience by disabling search suggestions in the location bar. IT seems to make sense to clearly separate contexts.

2. Though, as you said in comment 1, PB is about local traces. But we will enable tracking protection in PB, so it will lose that "specification" and become also a remote protection.

3. If we'd ever decide to remove the search bar (I'm guessing here, no plans I know about), then users will lose a valuable feature :(

4. Maybe we could switch to Yahoo in PB mode, that promised to never track Firefox users? (But I guess that agreement is only valid in the US and some countries don't have good yahoo experience afaik)

5. Should we disable it also for users in permanent PB? they would lose a valuable feature.

This needs discussion and a product management decision, imo.
Flags: needinfo?(mak77)
Flags: needinfo?(kev)
Flags: firefox-backlog+
Priority: -- → P1
Whiteboard: [suggestions][fxsearch]
(In reply to Marco Bonardo [::mak] from comment #3)
> 1. Until we still have the search bar, it's likely we can provide the best
> "private" experience by disabling search suggestions in the location bar. IT
> seems to make sense to clearly separate contexts.

I'm not sure what you're saying here.

> 2. Though, as you said in comment 1, PB is about local traces. But we will
> enable tracking protection in PB, so it will lose that "specification" and
> become also a remote protection.

True -- I forgot about tracking-protection-in-PB. That demonstrates that we are already aiming to reduce 3rd-party visibility in PB mode (contradicting my comment 1), and search suggestions also may be 
marginally less user-specific/useful. Both good reasons to turn off search suggestions.
  
> 3. If we'd ever decide to remove the search bar (I'm guessing here, no plans
> I know about), then users will lose a valuable feature :(

Without any concrete knowledge of plans for this, I don't think we should let this influence our decision here. But considering it for the moment:
 (a) Users can still just type words into the URLbar and hit enter (like they do in the searchbar). So, the wouldn't be losing too much - just the character-by-character prediction.
 (b) It's also not *too* hard to just go to their search-engine-of-choice and type stuff into a search box. :)
 (c) For comparison, Chrome (on desktop) and Firefox for Android are already living in a no-searchbar world, and they *do* disable search suggestions in PB mode. IMO the privacy benefits are well worth it even in that world. (Even Chrome has made this tradeoff.)

> 4. Maybe we could switch to Yahoo in PB mode, that promised to never track
> Firefox users? (But I guess that agreement is only valid in the US and some
> countries don't have good yahoo experience afaik)

Interesting idea, but I don't think this would come off well...

> 5. Should we disable it also for users in permanent PB? they would lose a
> valuable feature.

Probably, though it depends on why users are using permanent PB. But if we're enabling tracking protection for those users by default (which has nothing to do with local storage 100% about 3rd-parties-seeing-what-you're-doing), then I think it makes just as much sense to turn off this feature for those users as well.

Let's also not oversell the criticality of this feature -- it's a convenience, but often it just saves a few keystrokes, and users can get the same convenience by just going directly to a search engine (or by searching from the URLbar once, not being happy with the results, and adjusting the search terms on the results page that they landed on).

Also, I'd bet that permanent-PB users make up a small enough fraction of our users that we shouldn't wring our hands too much about how bad it is for them to get a slightly-less-useful-but-more-user-privacy-respecting behavior here.

(We could potentially revisit the permanent-PB decision later on, too, after user research or whatever. For now, I'm mainly concerned about an occasional PB user being surprised by this & losing faith in Firefox as a privacy-respecting browser.)

> This needs discussion and a product management decision, imo.

Yup. It would be useful to find where we had this discussion on Firefox for Android. IMO it would take a very compelling case for us to have different behavior on this point, on Android vs. Desktop.
landed private browsing window - but isn't working - so need to get resolve the private browsing more working 

Kev will look into PM decision on how we want this to work in permanent browsing mode.
Assignee: nobody → adw
Iteration: --- → 42.2 - Jul 27
Rank: 17
It's definitely a bug that suggestions appear in private windows.  That's not the intended behavior as implementing in bug 959594.  Permanent PB mode is something separate and I'll focus this bug on the former.
Status: NEW → ASSIGNED
Blocks: 959594
(In reply to Drew Willcoxon :adw from comment #6)
> That's not the intended behavior as implementing in bug 959594.

as implemented
Attached patch patchSplinter Review
I'm wrong, disabling suggestions in private windows was not the intended behavior in bug 959594.  I got mixed up because we do indeed handle private windows differently, but only by passing a private flag to the search suggestion controller, which then does xhr.channel.setPrivate(true) when fetching suggestions.

Here's a patch that does disable suggestions in private windows.  It continues passing _inPrivateWindow to the suggestions controller, which I think still the correct thing to do even though it's a moot point now.
Attachment #8633564 - Flags: review?(mak77)
Comment on attachment 8633564 [details] [diff] [review]
patch

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

So, this disables suggestions in both private windows and permanent PB mode, otherwise if we'd want to keep them enabled in permanent PB we should use _disablePrivateActions (yeah, var names suck, especially when there's no documentation above them).

I think for now this is fine, we can re-evaluate permanent PB in future, we first need some telemetry about the number of users in permanent pb, also for the great or dead effort.
Attachment #8633564 - Flags: review?(mak77) → review+
PS: it's not critical since this is trivial, but if you want to add a mini test to the xpcshell suggestions test, you can pass a searchParam property to check_autocomplete.
Flags: needinfo?(kev)
https://hg.mozilla.org/integration/fx-team/rev/0f8643a296a4

I added the test you suggested and also another test to make sure suggestions are disabled when browser.urlbar.suggest.searches=true but browser.search.suggest.enabled=false.

I also added a comment above Search() to explain its parameters, including searchParam.  We can tweak its wording as you'd like in future patches.
thanks, LGTM.
https://hg.mozilla.org/mozilla-central/rev/0f8643a296a4
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Flags: qe-verify+
QA Contact: petruta.rasa
Verified that there are no suggestions in Private Windows and Permanent private-browsing mode using Nightly 42.0a1 2015-07-21 across platforms.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.