Closed Bug 567121 Opened 14 years ago Closed 13 years ago

History is not cleared immediately after selecting Clear in Options->Preferences

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 6

People

(Reporter: ahoza, Assigned: mfinkle)

References

Details

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3
Build Identifier: Mozilla/5.0(X11;U;Linux armv7l; Nokia N900; en-US;rv:1.9.2.5pre) Gecko/20100520 Namoroka/3.6.5pre Fennec 1.1b2pre 

Previously visited sites are still available after clearing private data.

Reproducible: Always

Steps to Reproduce:
1. Clear private data and restart browser
2. Visit www.google.com and then mozilla.org
3. Go to Options->Preferences and tap on Clear button in "Clear private data section"
4. Go back to the awesome bar.
5. Start typing "goo"

Actual Results:  
At step 4, previously visited sites are also available in the list. When starting typing "goo", empty set is displayed and if the bar is cleared those sites are not available anymore in the list.

Expected Results:  
When performing step 4 only bookmarked pages are displayed (no history).
Empty result set is displayed after step 5.

The following bug was filed describing the same behavior: https://bugzilla.mozilla.org/show_bug.cgi?id=547993
The problem with this bug is the AutoCompleteCache is being lazily initialized.

I can't reproduce this bug as stated in comment 0. As soon as I open the awesome bar the AutoCompleteCache is initialized and clearing the cache works.

But if I open Fennec and go directly to clear the cache (don't open the awesomebar) then the cache is not cleared.

Bug 547993 never saw this issue because we always opened the awesome bar when Fennec loaded, therby initing the AutoCompleteCache.

I don't think this is a big deal, now that I see the problem. We could always init the AutoCompleteCache during startup, but that might hurt Ts.

We could init the AutoCompleteCache in our delayed startup code.

Thoughts?
Let me just point out that the session history is _always_ clear as soon as you press the button. What you see in the awesombar is just some left over items stuck in the AutoComplete cache, waiting for the cache to be cleared.

The next time you open the awesomebar, or even clear the text in the bar and start typing again, the cache will be cleared to reflect the history.
Assignee: nobody → mark.finkle
Status: UNCONFIRMED → NEW
Ever confirmed: true
Mozilla/5.0 (Android; Linux armv71; rv2.0b13pre) Gecko/20110310 Firefox/4.0b13pre Fennec/4.0b6pre
Device: Droid 2 
OS: Android 2.2

After I hit the clear button in today's build (with or without sync) I get an error in console:

Error: [Exception... "JavaScript component does not have a method named: "observe" when calling method: [nsIObserver::observe]" nsresult:"0x80570030 (NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED)" location: "JSframe :: chrome://broser/content/sanitize.js :: <TOP_LEVEL> :: line 195" data:no ]
Source File: chrome://browser/content/sanitize.js
Blocks: 642635
Attached patch Patch? (obsolete) — Splinter Review
Is there some reason we can't just do this?
Attachment #522530 - Flags: review?(mark.finkle)
Comment on attachment 522530 [details] [diff] [review]
Patch?

Deleting the file and setting the cache to null doesn't directly do anything. Setting the cache to null will cause this code path to be skipped though:

http://mxr.mozilla.org/mobile-browser/source/components/AutoCompleteCache.js#299

And we will force a fetch here:
http://mxr.mozilla.org/mobile-browser/source/components/AutoCompleteCache.js#313

If that is the desired outcome anyway, let's just change the existing AutoCompleteUtils.update() call to a AutoCompleteUtils.fetch(AutoCompleteUtils.query) call:
http://mxr.mozilla.org/mobile-browser/source/components/AutoCompleteCache.js#347

r- if we can drop the file exists/remove code and just call fetch directly
Attachment #522530 - Flags: review?(mark.finkle) → review-
Attached patch Patch (obsolete) — Splinter Review
Attachment #522530 - Attachment is obsolete: true
Attachment #522664 - Flags: review?(mark.finkle)
Comment on attachment 522664 [details] [diff] [review]
Patch

I assume this actually works? Any ideas for testing this?
Attachment #522664 - Flags: review?(mark.finkle) → review+
Yeah works fine.

I tested two ways, by looking at the cache file immediately after I clicked "Clear" in prefs, and by opening the urlbar to make sure that the old results weren't shown (there is a brief flash while we reset the autocomplete items, we could reset that as well).

We can do the same thing in tests I assume. i.e. Look at the file to make sure it is updated, and open the urlbar to make sure it has updated. Working on those....
Pushed: http://hg.mozilla.org/mobile-browser/rev/6b9a963ef505

I'll file a follow up to write some tests for sanitize (already in progress in my queue).
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
I can still reproduce this on:

Build ID: Mozilla /5.0 (Android;Linux armv7l;rv:2.2a1pre) Gecko/20110406 Firefox/4.2a1pre Fennec /4.1a1pre 

Device: Motorola Droid 2 (Android 2.

Opening the bug...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch Patch v2Splinter Review
Talked to sdwilsh about this today. The problem is that the PlacesAutocompleteService (PACs) holds a separate readonly connection to the places database than the normal history service. When the history service clears the history, PACs may not read from that exact same database.

Instead, he said to follow what they do in tests which need to be sure history has been cleared. These watch for a notification from places:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/tests/browser/browser_visituri_nohistory.js#46

This just updates our AutoCompleteCache service to use this same notification. I looked into changing other areas, but unless a service really needs to know when history has been cleared, and not just that the user has requested history be cleared, we should be fine. I didn't see any other services that really met that condition for me.
Attachment #522664 - Attachment is obsolete: true
Attachment #533354 - Flags: review?(mark.finkle)
Comment on attachment 533354 [details] [diff] [review]
Patch v2

Our tests still pass?
Attachment #533354 - Flags: review?(mark.finkle) → review+
Tests pass as well with as without for me. Pushed:
http://hg.mozilla.org/mozilla-central/rev/6f5cb026f6d8
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 6
VERIFIED FIXED on:

Build Id: Mozilla /5.0 (Android;Linux armv7l;rv:6.0a2) Gecko/20110705 Firefox/6.0a2 Fennec/6.0a2 

Device: HTC Desire Z (Android 2.2)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: