657263 - xulstore is keeping a quasi history via place: and find: urls in RDF:about attributes

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect, P3)

Description

[al_9x]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.17) Gecko/20110420 Firefox/3.6.17
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0.1) Gecko/20100101 Firefox/4.0.1

Fx 3.6 & 4

this "history" is not cleared through normal means
There is a claim in this thread:
http://forums.mozillazine.org/viewtopic.php?f=7&t=1601405

that this is junk left over from previous versions, but that's not the case.

I noticed one source of the place: urls, history sidebar selections.  But I think there are others, as that does not explain all the entries.  Not sure what causes the find: urls to be added.

When history is cleared (per site or completetely) these entries should be purged.  

Reproducible: Always

Comment 1

[Marco Bonardo [:mak]]

this comes from far before 3.6, I'd say from 3.0.
the place: uris there are saved to remember the "open/closed" status of  containers, the ones that could be a privacy problem could be the site containers.
I'd be suprised if there wouldn't be a dupe for this.

Comment 2

[al_9x]

What purpose do the "find:" URIs serve?

Since this functionality produces effectively a listing of visited sites, it should be cleared with history.

Comment 3

[(mostly gone) XtC4UaLL [:xtc4uall]]

(In reply to comment #1)
> I'd be suprised if there wouldn't be a dupe for this.

I found Bug 317260/Bug 251874 and also several Core/XUL Bugs what mention persisted Attributes. No Idea if they are relevant since I have only place:/chrome: URIs stored.

Comment 4

[Marco Bonardo [:mak]]

(In reply to comment #3)
> I found Bug 317260/Bug 251874 and also several Core/XUL Bugs what mention
> persisted Attributes. No Idea if they are relevant since I have only
> place:/chrome: URIs stored.

the site containers ARE place: uris.

Comment 5

[Justin Dolske [:Dolske]]

How would a user have gotten these set? Expanding folders in the places organizer?

Comment 6

[Marco Bonardo [:mak]]

Or in the sidebar, anything using the treeview

Comment 7

[al_9x]

What is the source of "find:" URIs?

Comment 8

[Marco Bonardo [:mak]]

no reason to stay unconfirmed

Comment 9

[Marco Bonardo [:mak]]

we should use hash of the node uri and/or the guid, not the node uri itself.

Comment 10

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.

Comment 11

[Marco Bonardo [:mak]]

This still applies to XULStore instead of RDF, we should hash place: uris before storing them into place:{base64-hash}. It would be nice to also convert or remove existing entries cause they'll stop working, the xulstore API can probably be used for that.
I'm not sure what's the source of find: urls, I couldn't find it in code, so the fix may also involve some code archeology to understand who was/is creating them.

Comment 12

[Jane Kotovich (:janey)]

Would love to work on this one!

Comment 13

[Marco Bonardo [:mak]]

This search may help investigating the call points: https://searchfox.org/mozilla-central/search?q=xulstore&path=places
We should make them store a hash instead of the url, and as I said in comment 11, I'd probably retain the place: protocol in front just to clarify what those urls are about.
You should probably create a PlacesUIUtils.obfuscateUrlForXulStore(url) that will receive the current place: uri as argument and returns a place:[base64_hash], then getValue, setValue and removeValue would use that.
You should also add a migration to migrateUI https://searchfox.org/mozilla-central/rev/2e3b0483e31abffe0b4374480a34c6d23f5186ea/browser/components/BrowserGlue.jsm#3303,3931 that should go through all the uris stored and convert them to the new format:
Example

Array.from(Services.xulStore.getIDsEnumerator("chrome://browser/content/browser.xhtml")).filter(id => id.startsWith("place:")).forEach(id => {
  console.log(id);
});

Same for "chrome://browser/content/places/bookmarksSidebar.xhtml" and "chrome://browser/content/places/historySidebar.xhtml"
Here is an example for md5 https://searchfox.org/mozilla-central/rev/2e3b0483e31abffe0b4374480a34c6d23f5186ea/browser/components/newtab/lib/FilterAdult.jsm#32-50

Comment 14

[Marco Bonardo [:mak]]

I honestly don't know if there's a test for the functionality, my initial search didn't reveal anything. The idea behind this is that if you close the bookmarks sidebar with a certain set of folders open, then close and reopen it, the open state of those folders is restored.
One way to check if there's a test may be to break the code doing it and see what fails. Otherwise we may have to add one.

Comment 15

[Jane Kotovich (:janey)]

Attachment: Bug 657263: xulstore is keeping a quasi history via place: urls. r=mak

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d2b5e7cc2dbb
xulstore is keeping a quasi history via place: urls. r=mak

Comment 17

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/d2b5e7cc2dbb

Comment 18

[Bob Clary [:bc] (inactive)]

I'm not sure if this patch is responsible, but it seems it may be related. After my Nightly / Linux x86_64 updated to 20211112213406 from 20211112092317, I lost all of my tabs and had to go back to the previous sessionstore backup to recover them. I was also logged out of my Firefox account fwiw.

Comment 19

[Marco Bonardo [:mak]]

(In reply to Bob Clary [:bc] (inactive) from comment #18)

I'm not sure if this patch is responsible, but it seems it may be related. After my Nightly / Linux x86_64 updated to 20211112213406 from 20211112092317, I lost all of my tabs and had to go back to the previous sessionstore backup to recover them. I was also logged out of my Firefox account fwiw.

Please check bug 1740987, the fix is in the newest Nightly, if updates should not work, you may have to manually reinstall over Nightly.