Search bar should not save search history in private windows

RESOLVED FIXED in seamonkey2.20

Status

SeaMonkey
Search
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: neil@parkwaycc.co.uk)

Tracking

(Blocks: 1 bug)

Trunk
seamonkey2.20
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
Steps to reproduce problem:
1. Open a private window
2. Customise the toolbar to show the search bar
3. Perform a search
4. Press down arrow

Actual results: Search just performed appears in search history

Expected results: Search history unaffected by searches in private windows
(Assignee)

Comment 1

4 years ago
Created attachment 737311 [details] [diff] [review]
Possible patch

While I was looking into this I noticed that the search backend (nsSearchSuggestions.js) expects the autocomplete search param to have a |private suffix when the window is private, so that it can use private channels. However the problem then appears to arise that the code to clear search suggestions (which I presume should continue to function in private windows) does not want the |private suffix. (Firefox doesn't do this, so I don't know whether it gets this wrong here, or whether I have misread the code.) To work around this I have simply hardcoded the search param when dealing with the search history.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #737311 - Flags: feedback?(philip.chee)
Attachment #737311 - Flags: feedback?(iann_bugzilla)
(Assignee)

Updated

4 years ago
Summary: Search bar should not save suggestions in private windows → Search bar should not save search history in private windows
(In reply to neil@parkwaycc.co.uk from comment #1)
> (Firefox doesn't do this, so I don't know whether it gets this wrong here, or
> whether I have misread the code.)

In the Firefox version of search.xml, the "private" suffix is only added by the JS getter (.searchParam, read by the autocomplete controller), the DOM attribute itself is left as-is.
(Assignee)

Comment 3

4 years ago
(In reply to Gavin Sharp from comment #2)
> (In reply to comment #1)
> > (Firefox doesn't do this, so I don't know whether it gets this wrong here, or
> > whether I have misread the code.)
> 
> In the Firefox version of search.xml, the "private" suffix is only added by
> the JS getter (.searchParam, read by the autocomplete controller), the DOM
> attribute itself is left as-is.

Subtle! Turns out it was my idea (bug 640427 comment 2, but only one version got changed) to switch SeaMonkey's "clear search autocomplete" to use the getter instead of the attribute...

Comment 4

4 years ago
Comment on attachment 737311 [details] [diff] [review]
Possible patch

> +        if (this.usePrivateBrowsing)
> +          this._textbox.searchParam += "|private";

> -              textBox._formHistSvc.addEntry(textBox.getAttribute("autocompletesearchparam"),
> -                                            textValue);
> +              textBox._formHistSvc.addEntry("searchbar-history", textValue);

> -              return this._formHistSvc.nameExists(this.searchParam);
> +              return this._formHistSvc.nameExists("searchbar-history");

> -              var param = this.getAttribute("autocompletesearchparam");
> -              this._formHistSvc.removeEntriesForName(param);
> +              this._formHistSvc.removeEntriesForName("searchbar-history");

Instead of hardcoding for "searchbar-history" can we (in the constructor) cache the original autocompletesearchparam and reuse this?

Comment 5

4 years ago
Comment on attachment 737311 [details] [diff] [review]
Possible patch

f=me
Attachment #737311 - Flags: feedback?(philip.chee) → feedback+

Comment 6

4 years ago
Comment on attachment 737311 [details] [diff] [review]
Possible patch

Should switching to a different search engine within the private window be remembered?
Attachment #737311 - Flags: feedback?(iann_bugzilla) → feedback+

Comment 7

4 years ago
> Should switching to a different search engine within the private window be remembered?
What does Firefox do in this case?

Comment 8

4 years ago
>> Should switching to a different search engine within the private window be remembered?
> What does Firefox do in this case?
As far as I can tell, Firefox doesn't check the private browsing status when switching to a different search engine.
(Assignee)

Updated

4 years ago
Attachment #737311 - Flags: review?(philip.chee)

Comment 9

4 years ago
Comment on attachment 737311 [details] [diff] [review]
Possible patch

Discussed this over IRC.
r=me
Attachment #737311 - Flags: review?(philip.chee) → review+
(Assignee)

Comment 10

4 years ago
Pushed comm-central changeset 3658f802883b.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED

Updated

4 years ago
Target Milestone: --- → seamonkey2.20
(Assignee)

Comment 11

4 years ago
Comment on attachment 737311 [details] [diff] [review]
Possible patch

[Approval Request Comment]
Regression caused by (bug #): N/A
User impact if declined: Private search information leak
Testing completed (on m-c, etc.): Landed on c-c a few days ago
Risk to taking this patch (and alternatives if risky): Low
String changes made by this patch: None
Attachment #737311 - Flags: approval-comm-release?
Attachment #737311 - Flags: approval-comm-beta?
Attachment #737311 - Flags: approval-comm-aurora?

Updated

4 years ago
Attachment #737311 - Flags: approval-comm-beta?
Attachment #737311 - Flags: approval-comm-beta+
Attachment #737311 - Flags: approval-comm-aurora?
Attachment #737311 - Flags: approval-comm-aurora+

Updated

4 years ago
Attachment #737311 - Flags: approval-comm-release?
You need to log in before you can comment on or make changes to this bug.