On a clean profile : browsing to several sites : the list does appear in History menu, but when using Display > Sidebar > History : no sites are shown. Clicking on the Sort button (default is By Date radio button), then choosing By site, sites list appears. Mozilla/5.0 (X11; U; Linux i686; fr; rv:1.9b4pre) Gecko/2008022904 Minefield/3.0b4pre

I'm not able to reproduce this on latest nightly. do you see the "Today" folder? If so, is the expected history contained in the contents of that folder?

This still occurs on latest fr tinderbox build (Mozilla/5.0 (X11; U; Linux i686; fr; rv:1.9b4pre) Gecko/2008030304 Minefield/3.0b4pre) but not on en-US latest nightly, where the folder "Today" appears. So, I guess I should requalify this bug to a fr l10n bug?

(In reply to comment #2) > So, I guess I should requalify this bug to a fr l10n bug? > sounds like it, yes. thanks for checking.

I saw there was no line at the end of file l10n/fr/toolkit/chrome/places/places.properties. I make a try with adding one.

I reproduce this bug on Windows XP (I don't know if it's the same on Mac). I think I found what is wrong. In the l10n/fr/toolkit/chrome/places/places.properties file, the string finduri-AgeInDays-is-0=Aujourd'hui contains an apostrophe. When I removed it, the bug disappeared; when I tried to escape it with a backslash (\'), the bug came back. I even tried to surround the string with quotes, same bug. I don't know how to fix this and I guess this is a l12y problem. Could someone help me here?

Changing the old summary "History not shown in sidebar with clean profile on first launch" to make it more accurate

I'm not sure it is relevant, but it seems that our string (finduri-AgeInDays-is-0) is passed unescaped to an SQL query here: http://mxr.mozilla.org/mozilla/source/toolkit/components/places/src/nsNavHistory.cpp#2993 Since "Aujourd'hui" contains a quote character ('), I think this would cause a parse error around the dayTitle field when executing the SQL query.

(In reply to comment #7) > I'm not sure it is relevant, but it seems that our string > (finduri-AgeInDays-is-0) is passed unescaped to an SQL query here: > http://mxr.mozilla.org/mozilla/source/toolkit/components/places/src/nsNavHistory.cpp#2993 > > Since "Aujourd'hui" contains a quote character ('), I think this would cause a > parse error around the dayTitle field when executing the SQL query. > OOPS. Thanks for analysis. I will fix it.

Why are we dynamically building a query string anyway? The reason for the bind parameter api's exactly because of things like this...

Shawn, can you file a follow up on the issue from comment #9? In the meantime, this goes to l10n::fr ...

(In reply to comment #10) > Shawn, can you file a follow up on the issue from comment #9? > > In the meantime, this goes to l10n::fr ... > I'm afraid it is really my problem in Places. Localizers cannot do anything better and it could be related to multiple languages. I will try to solve it with binding.

(In reply to comment #10) > Shawn, can you file a follow up on the issue from comment #9? Filed bug 421115

(In reply to comment #10) > In the meantime, this goes to l10n::fr ... This is a Places bug...

This patch refactors the way parameters are bound in history service. It replaces position based parameters with named parameters. This makes the code more robust and is much more readable. For instance SelectAsSite query: - "&beginTime='||?1||'&endTime='||?2, " + "&beginTime='||:begin_time||'&endTime='||:end_time, " Previously it was not easy to find what does ?1 refer to, now it is obvious. Thanks to this it is now possible to add additional named parameters in PlacesSQLQueryBuilder. So the day names can now be bound rather then injected to the SQL string. French and all other locals should work fine now.

Comment on attachment 307736 [details] [diff] [review] Refactor and enhance parameter binding I found one more candidate for binding.

Changes since first patcy: - binding added for (local) - fixed incorrect sql for uriIsPrefix (tested using parts of bug 384226)

Comment on attachment 308400 [details] [diff] [review] Refactor and enhance parameter binding v2 nice win on readability and compactness in the parameter bindings, as well as removing the fragility of the parameter order requirement (albeit trading off for more complexity in the helpers). can you please confirm that we have a test that exercises the multiple query functionality? if not, please add one. also, please add a test specific to the problem this bug addresses. r=me otherwise. > nsresult ConstructQueryString(const nsCOMArray<nsNavHistoryQuery>& aQueries, > nsNavHistoryQueryOptions *aOptions, > nsCString& queryString, >- PRBool& aParamsPresent); >+ PRBool& aParamsPresent, >+ StringHash & aAddParams); nit: remove space

I have modified places.properties locally to include single quote and saw that it failed without this patch. With patch applied it was shown correctly. I'm not sure if I can create a test that would address this bug. It is covered with test_history_sidebar.js and would be visible (the query would fail), if the tests were executed with fr-FR locale. I would prefer if multiple queries and complex queries get tested with bug 384226. I can take that bug over, but it is not blocking.

I have added a simple test for multiple queries - and it did not PASS. Current code does this: WHERE original AND additional1 OR additional2 What is interpreted as: WHERE (original AND additional1) OR additional2 It must be changed to: WHERE original AND (additional1 OR additional2) It is actually another bug, but I hope we can fix it with this patch.

Comment on attachment 310516 [details] [diff] [review] Refactored parameter binding + multi query test + fix for it the tests win yet again ;) r=me.

Checking in toolkit/components/places/src/nsNavHistory.cpp; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.cpp,v <-- nsNavHistory.cpp new revision: 1.277; previous revision: 1.276 done Checking in toolkit/components/places/src/nsNavHistory.h; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.h,v <-- nsNavHistory.h new revision: 1.150; previous revision: 1.149 done RCS file: /cvsroot/mozilla/toolkit/components/places/tests/unit/test_multi_queries.js,v done Checking in toolkit/components/places/tests/unit/test_multi_queries.js; /cvsroot/mozilla/toolkit/components/places/tests/unit/test_multi_queries.js,v <-- test_multi_queries.js initial revision: 1.1 done

verified with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008032106 Minefield/3.0b5pre and Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-US; rv:1.9b5pre) Gecko/2008032104 Minefield/3.0b5pre

In member function ‘PRUint32 IndexGetter::For(const char*)’: http://mxr.mozilla.org/mozilla/source/toolkit/components/places/src/nsNavHistory.cpp#4837 warning: negative integer implicitly converted to unsigned type

In member function ‘nsresult nsNavHistory::ConstructQueryString(const nsCOMArray<nsNavHistoryQuery>&, nsNavHistoryQueryOptions*, nsCString&, PRBool&, nsDataHashtable<nsCStringHashKey, nsCString>&)’: http://mxr.mozilla.org/mozilla/source/toolkit/components/places/src/nsNavHistory.cpp#3298 warning: unused variable ‘clauseParameters’

I should have filed a new bug instead of reopening, sorry for that. See bug 424577.

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h". In Thunderbird 3.0b, you do that as follows: Tools | Message Filters Make sure the correct account is selected. Click "New" Conditions: Body contains places-to-b-and-h Change the action to "Delete Message". Select "Manually Run" from the dropdown at the top. Click OK. Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter. Gerv