1187606 - enter on the about:home and about:newtab search fields should search immediately even if suggestions haven't been received yet

Status: RESOLVED DUPLICATE of bug 1093571

(Firefox :: Search, defect, P3)

Description

[Marco Castelluccio [:marco]]

Steps to reproduce:
1) Write something in the about:home search field
2) Press "enter" or click on the button next to the text field

The results page should be opened at this point, but it isn't. It is opened only after I press "enter" (or click on the button) several times.

I see the following message in the Browser Console:
> SearchSuggestionController: HTTP Timeout

Comment 1

[Marco Castelluccio [:marco]]

I've noticed that if I wait a few seconds, the results page is opened.
The search field on about:home is the only one exhibiting this problem

Comment 2

[Marco Castelluccio [:marco]]

I've just tested this on another machine running OS X and I can reproduce it.

Comment 3

[Florian Quèze [:florian]]

"SearchSuggestionController: HTTP Timeout" is a message that indicates we failed to receive suggestions from the search engine. My guess when reading this bug is that the <enter> key doesn't work until we have received (or stopped waiting for) suggestions.

Note: I haven't tried to reproduce myself.

Comment 4

[Marco Castelluccio [:marco]]

I'd expect the search to start immediately if I press "enter" (because if I press "enter" I've made the decision not to wait for the suggestions).

This bug makes searching from about:home basically unusable for me.

Comment 5

[(inactive) Nihanth Subramanya [:nhnt11]]

I think this bug should have gone away with the new in-content search UI. Marco, are you testing on Nightly or Release?

Comment 6

[Marco Castelluccio [:marco]]

I'm using Nightly. The bug is particularly noticeable when the network conditions aren't good.

Comment 7

[Marco Castelluccio [:marco]]

The error is reported in this function:

  _dedupeAndReturnResults: function(suggestResults) {
    if (this._searchString === null) {
      // _searchString can be null if stop() was called and remote suggestions
      // were disabled (stopping if we are fetching remote suggestions will
      // cause a promise rejection before we reach _dedupeAndReturnResults).
      return null;
    }

    let results = {
      term: this._searchString,
      remote: [],
      local: [],
      formHistoryResult: null,
    };

    for (let result of suggestResults) {
      if (typeof result === "string") { // Failure message
        Cu.reportError("SearchSuggestionController: " + result);

Comment 8

[Marco Castelluccio [:marco]]

I can reproduce on Windows with Firefox Aurora as well.

Comment 10

[:glob ✱]

attachment 8670097 [details] holds my recording of the end user experience of this issue.

(In reply to Marco Castelluccio [:marco] from comment #6)
> I'm using Nightly. The bug is particularly noticeable when the network
> conditions aren't good.

.. or if you live somewhere with high latency to your search provider (see bug 1211779 for my timings from australia).

Comment 11

[:glob ✱]

Attachment: 1187606.patch

no idea if this is the right approach :)

this patch causes search events to be processed ahead of all others in the queue.

Comment 12

[Drew Willcoxon :adw]

Comment on attachment 8673470 [details] [diff] [review]
1187606.patch

Review of attachment 8673470 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the patch.  This bug looks like a duplicate of bug 1093571.  Judging by what I said there in comment 9, and I still think that's right, I don't think we should take this patch because it break how messages are handled in FIFO order.  Would you like to write a patch that does what that comment says?  It would be a bit more work.

Comment 13

[:glob ✱]

(In reply to Drew Willcoxon :adw from comment #12)
> Thanks for the patch.  This bug looks like a duplicate of bug 1093571.

i'll mark it as such.

> Judging by what I said there in comment 9, and I still think that's right, I
> don't think we should take this patch because it break how messages are
> handled in FIFO order.  Would you like to write a patch that does what that
> comment says?  It would be a bit more work.

sounds interesting, will give it a go.