The temp table query rewrite caused matchOnlyTyped to be checked for plain queries. We can create a special lazy query as well as dynamically process it for the other special queries.

Check for typed dynamically. No need to regenerate the query on preference change either. Passes tests, but no typed tests exists yet..

Arguably restricting on typed is more restrictive than restricting to history ;)

Comment on attachment 346747 [details] [diff] [review] v1.1 first-pass: diff --git a/toolkit/components/places/src/nsNavHistoryAutoComplete.cpp b/toolkit/components/places/src/nsNavHistoryAutoComplete.cpp --- a/toolkit/components/places/src/nsNavHistoryAutoComplete.cpp +++ b/toolkit/components/places/src/nsNavHistoryAutoComplete.cpp @@ -133,7 +133,7 @@ // not be read later and we don't have an associated kAutoCompleteIndex_ aQuery = NS_LITERAL_CSTRING( "SELECT h.url, h.title, f.url") + BOOK_TAG_SQL + NS_LITERAL_CSTRING(", " - "h.visit_count, h.frecency " + "h.visit_count, h.typed, h.frecency " "FROM moz_places_temp h " "LEFT OUTER JOIN moz_favicons f ON f.id = h.favicon_id " "WHERE h.frecency <> 0 " @@ -141,7 +141,7 @@ "UNION ALL " "SELECT * FROM ( " "SELECT h.url, h.title, f.url") + BOOK_TAG_SQL + NS_LITERAL_CSTRING(", " - "h.visit_count, h.frecency " + "h.visit_count, h.typed, h.frecency " "FROM moz_places h " "LEFT OUTER JOIN moz_favicons f ON f.id = h.favicon_id " "WHERE h.id NOT IN (SELECT id FROM moz_places_temp) " @@ -149,7 +149,7 @@ "{ADDITIONAL_CONDITIONS} " "ORDER BY h.frecency DESC LIMIT (?2 + ?3) " ") " - "ORDER BY 8 DESC LIMIT ?2 OFFSET ?3"); + "ORDER BY 9 DESC LIMIT ?2 OFFSET ?3"); while there it could be better adding a comment "// h.frecency" on the order by line we should have done that in other places too, this went forgotten +// nsNavTyped::GetDBAutoCompleteTypedQuery() +// +// Returns the auto complete statement used when autocomplete results are +// restricted to history entries. nit: restricted to HISTORY? +mozIStorageStatement* +nsNavHistory::GetDBAutoCompleteTypedQuery() +{ + if (mDBAutoCompleteTypedQuery) + return mDBAutoCompleteTypedQuery; + + nsCString AutoCompleteTypedQuery; + GetAutoCompleteBaseQuery(AutoCompleteTypedQuery); + AutoCompleteTypedQuery.ReplaceSubstring("{ADDITIONAL_CONDITIONS}", + "AND h.typed = 1"); + nsresult rv = mDBConn->CreateStatement(AutoCompleteTypedQuery, + getter_AddRefs(mDBAutoCompleteTypedQuery)); + NS_ENSURE_SUCCESS(rv, nsnull); + + return mDBAutoCompleteTypedQuery; +} I'm unsure if this should be a lazy statement at all, i mean: we tried to make lazy all special statements that are less likely to be used after browser start. If you want to provide the change in bug 426864 a user could most likely open the browser and open the locationbar dropdown, triggering this statement and having a laggish behaviour. So i'm unsure if this should be lazy or not. // restrictive query first mDBCurrentQuery = mRestrictTag ? GetDBAutoCompleteTagsQuery() : mRestrictBookmark ? GetDBAutoCompleteStarQuery() : + mRestrictTyped ? GetDBAutoCompleteTypedQuery() : mRestrictHistory ? GetDBAutoCompleteHistoryQuery() : static_cast<mozIStorageStatement *>(mDBAutoCompleteQuery); yeah this is difficult to guess, typed includes both history and bookmarks, so not sure what is more restrictive, hwv this is a good approximation imho. @@ -1017,7 +1041,8 @@ // any are violated, matchAll will be false. PRBool matchAll = !((mRestrictHistory && visitCount == 0) || (mRestrictBookmark && !parentId) || - (mRestrictTag && entryTags.IsEmpty())); + (mRestrictTag && entryTags.IsEmpty()) || + (mRestrictTyped && typed == 0)); i don't know if that would break our code styling, but having conditions aligned to first line would make this much more readable. This most likely needs a test though, if we had a test we would have not fallen in the error at first glance during query rewrite.

Adds tests and switch over to the default behavior pref.

Comment on attachment 346835 [details] [diff] [review] v2 >@@ -235,10 +234,12 @@ > pref("browser.urlbar.restrict.tag", "+"); > pref("browser.urlbar.match.title", "#"); > pref("browser.urlbar.match.url", "@"); >+pref("browser.urlbar.restrict.typed", "~"); nit: put this with the the other restrict prefs all else looks fine. the only worry i have is that the pref change will anger the already angry people when they upgrade and the awesomebar results change on them. but hopefully the increased control, and the prefs UI will assuage them.

once landed, please add dev-doc-needed so that the pref change gets documented.

Comment on attachment 346835 [details] [diff] [review] v2 This will effect 3.1 users as well, so we should land this on branch.

This actually should probably be a P2 blocker since we broke some of the customization (accidentally) that people use from 3.0 (if I'm reading this bug correctly)

Pushed a followup bustage fix to add mDBAutoCompleteTypedQuery to the array in FinalizeStatements. Thanks mak. http://hg.mozilla.org/mozilla-central/rev/ee25fdabc9fe

FYI, this landing on 1.9.1 without the SeaMonkey team knowing surprised us with a non-working UI pref and test failures, see bug 473734.

This looks entirely like a change to prefs. If that's true, then this is a non-issue doc-wise since there's already a bug for "create a complete guide to prefs". If there's anything other than a change to prefs here, re-apply the doc needed keyword.