315570 - Add a reusable autocomplete result type that doesn't depend on Mork

Status: RESOLVED FIXED

(Toolkit :: Autocomplete, enhancement)

Description

[Brett Wilson]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20051025 Firefox/1.5
Build Identifier: 

Current auto complete is a bit messy and the basic implementation depends on Mork, and all that is really needed is a way to set and store (value,comment) pairs. An implementation should be provided that makes it easy for random implementors to use without reimplementing the nsIAutoCompleteResult interface or using Mork.

Reproducible: Always

Steps to Reproduce:

Comment 1

[Brett Wilson]

Attachment: Patch to add simple autocomplete object to toolkit

This patch also changes the experimental (defaulted off) places history system to use the simple autocomplete result for URL autocompletion.

Comment 2

[Brett Wilson]

Attachment: Same as the previous one, but with obsolete files removed

Comment 3

[Brian Ryner (not reading)]

Comment on attachment 202283 [details] [diff] [review]
Same as the previous one, but with obsolete files removed

>+nsAutoCompleteSimpleResult::nsAutoCompleteSimpleResult()
>+{
>+}

This should initialize mDefaultIndex and mSearchResult.

>--- browser/components/places/src/nsNavHistory.cpp	8 Nov 2005 02:25:22 -0000	1.1
>+++ browser/components/places/src/nsNavHistory.cpp	8 Nov 2005 18:55:04 -0000
>@@ -1887,67 +1945,62 @@ nsNavHistory::StopSearch()
>-      NS_LITERAL_CSTRING("SELECT url,title FROM moz_history WHERE typed = 1 AND hidden <> 1 ORDER BY visit_count DESC LIMIT 1000"),
>+      NS_LITERAL_CSTRING("SELECT url,title FROM moz_history WHERE typed = 1 AND hidden <> 1 ORDER BY visit_count DESC LIMIT 100"),

This change seems unrelated...

Looks ok otherwise.

Comment 4

[Annie Sullivan]

Comment on attachment 202283 [details] [diff] [review]
Same as the previous one, but with obsolete files removed

looks good.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 202283 [details] [diff] [review]
Same as the previous one, but with obsolete files removed

>+  ~nsAutoCompleteSimpleResult() {};

Extra ";", fwiw.

Also, since it's not virtual, shouldn't it be private rather than protected?

>+  friend struct AutoCompleteIntermediateResultSet;

Some compilers (MSVC, I think) warn about the struct vs. class difference here.

Also, declaring something a friend that hasn't been declared has significant portability differences:  on some compilers it counts as a declaration at namespace scope, but in the standard I believe it doesn't.  For portability, it's probably better to declare |class AutoCompleteIntermediateResultSet;| outside the class definition before declaring it a friend inside the class definition.

Comment 6

[Brett Wilson]

Attachment: Style and portability fixed

Fix for dbaron's comments.