337178 - Land search suggestions code

Status: RESOLVED FIXED

(Firefox :: Search, enhancement)

Description

[Mike Connor [:mconnor]]

I thought this bug existed... guess not.

Need to land support for Search Suggestions, which is a Google Suggest-like feature that any search engine can support.  Details will be on the wiki soon for how other providers can support this.

Comment 1

[Mike Connor [:mconnor]]

Attachment: implement remote suggestion autocomplete

Discussed in triage call, want to take this for a2

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 221353 [details] [diff] [review]
implement remote suggestion autocomplete

- _createSuggestionURI needs to null-check suggestionURI
- wouldn't hurt to bump nsISearchEngine's UUID, but I guess it doesn't matter
- typo s/nsIAutoComleteResult/nsIAutoCompleteResult/
- sandboxHost is unused (meant to be used instead of "http://www.mozilla.org", I'm guessing)
- indent is weird around |+   this._textbox.mController.input = this._textbox;|
- don't use the os namespace for the added suggestionURL in the plugin files since it's not OpenSearch
- nit: a lot of trailing whitespace, s/ \n/\n/g ;)

Comment 3

[Mike Connor [:mconnor]]

Comment on attachment 221353 [details] [diff] [review]
implement remote suggestion autocomplete

approved at bon echo triage call

Comment 4

[Mike Connor [:mconnor]]

Attachment: what landed, with gavin's comments

Comment 5

[Mike Connor [:mconnor]]

Shazam.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Attachment: update installer manifests too

Comment 7

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 221365 [details] [diff] [review]
update installer manifests too

landed this on the branch.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix leak

Without this we leak all browser windows

Comment 9

[Mike Connor [:mconnor]]

Comment on attachment 221402 [details] [diff] [review]
Fix leak

can you add a comment saying we leak the world if we don't null this?  call it a hunch...

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix leak with comment

Comment 11

[:Gavin Sharp [email: [e-mail]]]

*** Bug 335439 has been marked as a duplicate of this bug. ***

Comment 12

[Peter van der Woude [:Peter6]]

Gavin, is there more to come ?
It doesn't work at all for Google/Yahoo (or any other searchengine) 

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20060515 Minefield/3.0a1 ID:2006051510 [cairo]
same result on a non-cairo build

Comment 13

[:Gavin Sharp [email: [e-mail]]]

Oh, looks like the installer manifest changes weren't landed, so installer builds won't work. I'll do that in a sec.

Comment 14

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/installer/windows/packages-static 	1.75
mozilla/browser/installer/unix/packages-static 	1.70

Comment 15

[Peter van der Woude [:Peter6]]

(In reply to comment #13)
>so installer builds won't work. I'll do that in a sec.
> 
I'm on .zip's 

Comment 16

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20060515 Minefield/3.0a1 ID:2006051514

verified

I filed 338064 for the pref issue

Comment 17

[Channy Yun [:channy]]

Search suggestion don't support 2byte language such as CJK right now. It's different to detect finishing syllables. e.x onKeyUp and onKeyDown in JavaScript. So Google made 2 byte langunage version. http://www.google.co.jp/webhp?complete=1&hl=ja

Comment 18

[Channy Yun [:channy]]

Attachment: Capture of bug on Google Suggestion with CJK typing

Comment 19

[Joe Hughes]

Channy, thanks for pointing this out (and making such a nice demo film!)--please open it as a separate bug rather than reopening this one.

Comment 20

[Channy Yun [:channy]]

I made a new bug https://bugzilla.mozilla.org/show_bug.cgi?id=338585