Not able to search bookmarks

[GoogleCodeExporter]

Promoted from 
http://tenfourfox.tenderapp.com/discussions/problems/213-cant-search-bookmarks

See issue ticket. Regression from 14.

Original issue reported on code.google.com by classi...@floodgap.com on 11 Sep 2012 at 5:11

[GoogleCodeExporter]

Regression window: 11.0b2 and 12.0 work. 13, 14, 15 and 15.0.1 don't. 

Original comment by chtru...@web.de on 11 Sep 2012 at 7:35

[GoogleCodeExporter]

I'll diff the changelogs between 12 and 13 later today. Thanks for the testing. 
I wonder why the OP said it worked for him in 14, though.

Original comment by classi...@floodgap.com on 11 Sep 2012 at 1:22

[GoogleCodeExporter]

(ftr, just tried it on my 7400 test system; 12 works, 13 doesn't, just like you 
said)

Original comment by classi...@floodgap.com on 11 Sep 2012 at 1:24

[GoogleCodeExporter]

> I wonder why the OP said it worked for him in 14, though.

I am the OP, what I actually said, somewhat ambiguously, was that it hadn't 
worked "since version 14", meaning that version 14 was the first version I'd 
tried that didn't work. (I somehow missed the release of 13 and so hadn't tried 
it.)

Sorry for the ambiguity 

Original comment by gltack...@gmail.com on 11 Sep 2012 at 1:47

[GoogleCodeExporter]

Testing this in aurora 17, searching history from the same window *does* work, 
but not searching bookmarks.

Original comment by classi...@floodgap.com on 19 Sep 2012 at 1:01

[GoogleCodeExporter]

Bookmark searching from the Awesome bar does work, except that it pulls in all 
the history stuff as well. Even if I delete the bookmarked page from history, 
the awesome bar will still retrieve it if it is in bookmarks. So that's a 
workaround at least, and does imply that the data store is not corrupt.

Original comment by classi...@floodgap.com on 19 Sep 2012 at 1:36

[GoogleCodeExporter]

applyFilters() and load() in browser/components/places/content/tree.xml allege 
that the offending function is "PlacesUtils.history.executeQueries" (and this 
appears to be 
toolkit/component/places/nsNavHistory.cpp:nsNavHistory::ExecuteQueries).
nsNavHistoryResult.cpp appears to be where the actual (non)population of the 
result occurs.

Original comment by classi...@floodgap.com on 19 Sep 2012 at 4:36

[GoogleCodeExporter]

Talked to Marco at Mozilla. In this SQL, logged through NSPR, this is wrong:

SELECT b.fk, h.url, COALESCE(b.title, h.title) AS page_title, h.rev_host, h.visi
t_count, h.last_visit_date, f.url, null, b.id, b.dateAdded, b.lastModified, b.pa
rent, (SELECT GROUP_CONCAT(t_t.title, ',') FROM moz_bookmarks b_t JOIN moz_bookm
arks t_t ON t_t.id = +b_t.parent  WHERE b_t.fk = b.fk AND t_t.parent = 4 ) AS ta
gs , h.frecency FROM moz_bookmarks b JOIN moz_places h ON b.fk = h.id LEFT OUTER
 JOIN moz_favicons f ON h.favicon_id = f.id WHERE NOT EXISTS (SELECT id FROM moz
_bookmarks WHERE id = b.parent 
AND parent = 4)  AND (( AUTOCOMPLETE_MATCH(  'inequality'  , h.url, page_title, 
tags,   0, 0, 0, 0, 4, 0)  AND  b.parent IN(  0  ,  0  ,  0  ,  0  ,  0  ,  0  ,
  0  ,  0  ,  0  ,  0  ,  0  )  AND  NOT h.url BETWEEN 'place:' AND 'place;' )) 
ORDER BY b.id ASC

Notice the parent IDs are all zero. The offending code is in nsNavHistory.cpp:

    clause.Condition("b.parent IN(");
    for (nsTArray<int64_t>::size_type i = 0; i < includeFolders.Length(); ++i) {
      clause.Str(nsPrintfCString("%d", includeFolders[i]).get());
      if (i < includeFolders.Length() - 1) {
        clause.Str(",");
      }
    }

The offending bug is M702639 which converted 32-bit parent IDs to 64-bit. This 
also affects AuroraFox, so adding Tobias.

Original comment by classi...@floodgap.com on 23 Sep 2012 at 7:07

[GoogleCodeExporter]

We should use "%" PRIi64 "..." for this instead of %d, since we're already 
requiring inttypes.h for the int64_t. Marco asked for this to be upstreamed, so 
I'll do that after I make sure this works.

Original comment by classi...@floodgap.com on 23 Sep 2012 at 7:15

[GoogleCodeExporter]

Actually, after fooling around with this in NSPR, a better solution that works 
is just to turn %d into %lld.

Original comment by classi...@floodgap.com on 23 Sep 2012 at 9:26

[GoogleCodeExporter]

https://bugzilla.mozilla.org/show_bug.cgi?id=793523

Original comment by classi...@floodgap.com on 23 Sep 2012 at 9:35

[GoogleCodeExporter]

Original comment by classi...@floodgap.com on 24 Sep 2012 at 4:58

• Changed state: Started

[GoogleCodeExporter]

Original comment by classi...@floodgap.com on 14 Oct 2012 at 12:52

• Changed state: Verified