Closed Bug 418712 Opened 12 years ago Closed 12 years ago

nsIAutoCompleteInput should fire an event when a search begins

Categories

(Toolkit :: Autocomplete, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9beta4

People

(Reporter: Dolske, Assigned: Dolske)

References

Details

(Keywords: addon-compat)

Attachments

(1 file, 1 obsolete file)

Attached patch Patch v.1 (obsolete) — Splinter Review
Currently, nsIAutoCompleteInput fires a "searchcomplete" event when the autocomplete search has finished.

There's no corresponding event for when a search begins. I had experimented with trying intercept the beginning with oninput, oncommand, onchange, etc, but because of the way input/searches can be delayed by timers, I wasn't getting right match. And, in any case, having a onsearchbegin event is semantically clearer.

[Adding late-compat, since this changes an existing interface that an extension might implement for some reason.]
Attachment #304611 - Flags: review?(gavin.sharp)
Assignee: nobody → dolske
Comment on attachment 304611 [details] [diff] [review]
Patch v.1

>Index: toolkit/components/autocomplete/public/nsIAutoCompleteInput.idl

Need to bump the UUID for this interface.

>   /*
>+   * Notification that the search has started
>+   */
>+  void onSearchBegin();

>Index: toolkit/components/autocomplete/src/nsAutoCompleteController.cpp

>+  // notify the input that the search is complete
>+  mInput->OnSearchBegin();

"That the search is starting"?
Attachment #304611 - Flags: review?(gavin.sharp) → review+
Attached patch Patch v.2Splinter Review
Review nits fixed.

a1.9? -- Small, low risk, doesn't change existing code/behavior other than to fire a new event.

This does add a new method to nsIAutoCompleteInput, so any extensions implementing it would need to add a stub interface. Doesn't seem like a very common thing for extensions to do, and the the breakage should be obvious if they do.
Attachment #304611 - Attachment is obsolete: true
Attachment #304618 - Flags: approval1.9?
(note that the patch is 20K, but that's because I'm modifying a bunch of existing tests to also test this new interface :-)
Comment on attachment 304618 [details] [diff] [review]
Patch v.2

a=beltzner for 1.9
Attachment #304618 - Flags: approval1.9? → approval1.9+
Checking in toolkit/components/autocomplete/public/nsIAutoCompleteInput.idl;
  new revision: 1.10; previous revision: 1.9
Checking in toolkit/components/autocomplete/src/nsAutoCompleteController.cpp;
  new revision: 1.77; previous revision: 1.76
Checking in toolkit/components/autocomplete/tests/unit/test_378079.js;
  new revision: 1.3; previous revision: 1.2
Checking in toolkit/components/autocomplete/tests/unit/test_393191.js;
  new revision: 1.2; previous revision: 1.1
Checking in toolkit/components/autocomplete/tests
  new revision: 1.2; previous revision: 1.1
Checking in toolkit/components/places/tests/unit/test_000_frecency.js;
  new revision: 1.5; previous revision: 1.4
Checking in toolkit/components/places/tests/unit/test_408221.js;
  new revision: 1.2; previous revision: 1.1
Checking in toolkit/components/places/tests/unit/test_413784.js;
  new revision: 1.3; previous revision: 1.2
Checking in toolkit/components/places/tests/unit/test_416214.js;
  new revision: 1.2; previous revision: 1.1
Checking in toolkit/components/places/tests/unit/test_frecency.js;
  new revision: 1.2; previous revision: 1.1
Checking in toolkit/components/places/tests/unit/test_history_autocomplete_tags.js;
  new revision: 1.2; previous revision: 1.1
Checking in toolkit/components/places/tests/unit/test_multi_word_search.js;
  new revision: 1.2; previous revision: 1.1
Checking in toolkit/components/satchel/src/nsFormFillController.cpp;
  new revision: 1.98; previous revision: 1.97
Checking in toolkit/content/widgets/autocomplete.xml;
  new revision: 1.118; previous revision: 1.117
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.