536081 - Can't delete all history entries returned by a search in Library or Sidebar

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Heather Arthur [:harth]]

When trying to clean up my awesomebar, I opened the library, filtered for urls based on 'facebook' and Ctrl-A then deleted. I noticed that several urls weren't deleted. I tried also deleting these urls individually, and while doing this made it disappear from the list, refiltering brought them back up. It seems these items can't be deleted. Found regression range on mozilla-central nightlies:

Last good nightly: 2009-10-05 First bad nightly: 2009-10-06

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=[phone]+00%3A00%3A00&enddate=2009-10-06+04%3A00%3A00

Comment 1

[Marco Bonardo [:mak]]

could you please eval in the error console this code: 

Components.utils.import("resource://gre/modules/PlacesDBUtils.jsm");PlacesDBUtils.checkAndFixDatabase();

after some seconds it should give you back some logging (if you want to push that here, could be useful).

Can you create steps to reproduce the issue in a new profile so that we can try to reproduce it from scratch? I want to be sure it is not due to some underlying database issue.

Comment 2

[Heather Arthur [:harth]]

INTEGRITY
ok
- - -
CLEANUP
- - -
VACUUM
places.sqlite: 7585792 byte
- - -
STATS
places.sqlite: 6283264 byte
moz_bookmarks: 153
moz_bookmarks_roots: 5
moz_keywords: 0
sqlite_sequence: 0
moz_favicons: 732
moz_annos: 42
moz_anno_attributes: 10
moz_items_annos: 43
moz_places: 9175
moz_historyvisits: 18683
moz_inputhistory: 429
- - -

I can't reproduce on a clean profile from scratch, but this does happen when I copy my places.sqlite to a clean profile, so it seems like it might be a database issue, though it's odd that it happens only on trunk.

Comment 3

[Marco Bonardo [:mak]]

the above check should have also cleaned up the db a bit, i don't see anything particularly strange in the data.
if you can still reproduce, you could try to compress your database, and if it assumes an acceptable size send it to me through mail, so we don't discover privacy sensitive data and i can check what's up.

Comment 4

[u338317]

I have this problem in Win7x64, FF3.6b5.
I can also reproduce with a clean profile.

Steps I take:
create a new profile
google for 'testing;this'
go to library
use the 'Search History' bar and filter to 'google'
select the url that the earlier googling generated and hit delete

at this point the url might vanish, but if I re-filter with the search function, it will still be there.


I can get rid of the url by using the 'Forget about this site' command.
I can also get rid of url by looking at it through today's history, selecting it from there and deleting it.

So, basically this problem appears to be only occurring when I use the 'Search History' function.

Comment 5

[Heather Arthur [:harth]]

hm, this might be related the bug 521544. That also only occurs when filtering.

Comment 6

[u338317]

These two bugs look the same to me.

Only difference being that in this one you didn't specify what kind of characters the urls had.
In all likelyhood, knowing facebook, the urls that you couldn't delete also had some unusual characters.

Comment 7

[avada]

I can confirm this. It seems to happen with both 3.6 betas and 3.7 alphas.
Also I can confirm that it seems to only happen when I search for something either in the library or the history sidebar. From the awesomebar I can delete them.
Not sure about the special characters. On one hand I can't delete "http://szotar.sztaki.hu/dict_search.php?L=ENG:HUN:EngHunDict&W=something&M=2&P=2&C=1&A=1&E=1" wich has a colon in it. On the other hand I can't delete "http://www.youtube.com/get_video?video_id=ACbdCrGbISo&t=vjVQa1PpcFORZkCx5RRuL61CPFPXdtgM_m9yMc9BBnA=&fmt=18" Wich has nothing special in it. Also with the links from the other bug ( "http://ja.wikipedia.org/wiki/メインページ" and "http://www.facebook.com/b.php?charset_test=%E2%82%AC,%C2%B4,%E2%82%AC,%C2%B4,%E6%B0%B4,%D0%94,%D0%84&k=100000010&o=8&sf=f" ) quite the opposite happens. The item doesn't disappear from the list but searching again shows that it was removed from the databese.

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

So is this a dupe of bug 521544? Not a privacy issue, I don't think, just a not so great polish/ux issue; doesn't block release.

Comment 10

[Marco Bonardo [:mak]]

worth some investigation, could be a regression or missing bit of bug 500391. that would happen only on search though, usual deleting works correctly.

Comment 12

[Mina Naguib]

I'm also able to replicate this using Firefox 3.6 running under Mac OS X Snow Leopard 10.6.2

Comment 13

[Umang]

Also, when some of the items do actually seem to get deleted, they're not always deleted. Clearing the search bar (inside the history sidebar) and typing the search criteria again shows you that some of the items are still there.

To reproduce: Search for keywords that this bug affects (i.e. a result that doesn't clear when Ctrl+A, Del doesn't clear everything) and delete the first three results. They seem to go. Now delete the last character from the search bar and type it again. The deleted items will reappear.

Comment 15

[Umang]

Any progress in finding and fixing the bug?

Comment 16

[Marco Bonardo [:mak]]

sorry for the delay, looking into it right now.

Comment 17

[Marco Bonardo [:mak]]

I can only reproduce the bug with steps in comment 4. At first glance looks like the uri we save to the DB is the encoded uri, while the uri returned by the node (part of the query results) is the unencoded uri. So we try to delete something different from what is in the DB. I have a unit test, but still need to figure out this better.

Comment 18

[Marco Bonardo [:mak]]

ok, i found the culprit change. patch coming.

Comment 19

[Marco Bonardo [:mak]]

Attachment: patch v1.0

So, this removes an unwanted NS_Unescape when searching, with a test.
The remaning changes are replacing url string bindings with a common method, since we stringHead urls, it should be done everywhere, and also allows to eventually apply same transformations to urls in future.

Comment 20

[Marco Bonardo [:mak]]

PS: most likely we want this on next 3.6.x

Comment 21

[Dietrich Ayala (:dietrich)]

Comment on attachment 432715 [details] [diff] [review]
patch v1.0

these feel like they should be in storage utils, not just in places. r=me for now though.

also, is the scenario described in the bug summary the only case in which this bug is hit?

if not, can you clearly enumerate all the points in the UI in which users could hit this? that'll make the case for branch inclusion all the stronger.

Comment 22

[Marco Bonardo [:mak]]

(In reply to comment #21)
> (From update of attachment 432715 [details] [diff] [review])
> these feel like they should be in storage utils, not just in places. r=me for
> now though.

yeah this is probably true, but that could be an issue, URIs depend on channel charset, so storing them is not easy as we do in Places (that actually is not the right way)

> also, is the scenario described in the bug summary the only case in which this
> bug is hit?

the bug hits when searching (in bookmarks or history sidebar, or library) because in other cases we don't touch this code path.

I think searching history to remove entries is a quite important task for privacy and for increasing quality of the awesomebar searches, that was practically impossible in the past due to slowness, but today on 3.6 you can use it even on large dbs.

Comment 23

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/97d112dc740c

Comment 24

[Marco Bonardo [:mak]]

Comment on attachment 432715 [details] [diff] [review]
patch v1.0

asking approval to land on 1.9.2, the change is not really dangerous, and fixes a common functionality and a privacy concern. Test provided.

Comment 25

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 432715 [details] [diff] [review]
patch v1.0

a=beltzner for 1.9.2.4

Comment 26

[Marco Bonardo [:mak]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/54e951398337

Comment 27

[Al Billings [:abillings - ex-MoCo]]

Is the best repro case for this the one in comment 4?

Comment 28

[Al Billings [:abillings - ex-MoCo]]

Deciding "yes" and verified for 1.9.2 with the case in comment 4 and
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.4pre) Gecko/20100412
Namoroka/3.6.4pre (.NET CLR 3.5.30729). The site is deleted now (and I verified
that it isn't deleted in 1.9.2.3 with the same steps).

Comment 29

[Marco Bonardo [:mak]]

Sorry for missing answer, yes comment 4 is the best.