413497 - Awesome bar should use a throbber to provide feedback that search is in progress

Status: VERIFIED FIXED

(Firefox :: Address Bar, enhancement)

Description

[Martin Meyer]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008011902 Firefox/3.0b3pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008011902 Firefox/3.0b3pre

There is currently no feedback from the Awesome Bar to let you know that it's still pondering what you've typed. It would be nice to have a line item (unselectable) which says "Awesome Bar is looking for items from history, bookmarks and starred items that match the text you typed" with a throbber next to it. When search is done, change the text to "Awesome Bar is done searching for matches to what you typed."

I think this would be a good feature since I currently have no idea if searching is on-going or if it really didn't find anything to match what I typed. Sometimes results take several seconds if I haven't used that URL recently.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Ria Klaassen (not reading all bugmail)]

Do you have that much history? Does this always happen or only after startup? 
Seems similar to bug 413457 (also Linux).

Comment 2

[Martin Meyer]

I don't think my history should be particularly large... I do notice a CPU spike when the bar starts searching (i.e. bug 413457) but it's only momentary when I start typing.

My issue is that sometimes results will be added to the list after I let it sit for a few seconds, or that there are no results at all for a few seconds. This delay is only about 2-4 seconds at most, but it is noticeable sometimes.

What could cause staggered results to return? Is the Awesome Bar result set made up of several queries? If so, maybe one query takes longer to complete than the others. If it's not multiple queries, how is it possible that not all results show up at the same time? Any thoughts?

Comment 3

[Alex Faaborg [:faaborg] (Firefox UX)]

Yes, we definitely need to provide feedback to the user that the search is still commencing.  Nominating for blocking.

Comment 4

[Alex Faaborg [:faaborg] (Firefox UX)]

The decision to place the awesome bar throbber on the site button is based on wontfixing 402968 (we wanted the throbber in this location to only mean one thing), and the design of spotlight on OS X.

Comment 5

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: spotlight throbber screen shot

Screen shot of how spotlight provides nice feedback.

Comment 6

[Justin Dolske [:Dolske]]

Note that the work in bug 402968 can probably be largely recycled for here... The remaining work would be to add some awesomebar code to set/clear a XUL element attribute when the searching starts/stops.

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Really, really want, and might upgrade to blocking if the responsiveness of awesomebar continues.

I wonder, though, if we want it on the site button or in a row that appears at the bottom of the richlistbox of results?

(# = favicon, @ = throbber)

initial state ...

 (   | some search terms                             v )
     |-------------------------------------------------|
     | @ Searching history and bookmarks ....          |
     '-------------------------------------------------'

first chunk comes in ...
 (   | some search terms                             v )
     |-------------------------------------------------|
     | # Some cool site                                |
     |   http://some.cool.site                         |
     |-------------------------------------------------|
     | # Some other cool site                          |
     |   http://other.cool.site                        |
     |-------------------------------------------------|
     | # Searching is fun                              |
     |   http://we.like.search                         |
     |-------------------------------------------------|
     | # Responsiveness++                              |
     |   http://curse.those.milliseconds               |
     |-------------------------------------------------|
     | @ Searching history and bookmarks ....          |
     '-------------------------------------------------'

Comment 8

[Alex Faaborg [:faaborg] (Firefox UX)]

Totally agree, I especially like how this gives us a second opportunity to say "bookmarks and history" to reinforce what is being searched, and that this search is local to your computer.

Comment 9

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: "target acquired" throbber idea

This throbber would simply rotate, creating a cross hair with whitespace.  The idea is to give the awesome bar a missile lock-like feel, similar to a flight simulator.  I think it is also important to use a different throbber since the current one is strongly associated with network activity.

Comment 10

[Justin Dolske [:Dolske]]

Attachment: Patch v.1

This resurrects some of the work done in bug 402968 (throbber in URL bar), and activates it when searching (instead of loading pages). I've only made the required theme changes for OS X (so far).

Comment 11

[Justin Dolske [:Dolske]]

Bah, meant to add:

This patch was easy to do, since I has basically already done all the work in the other but (and it went through a few rounds of reviews). I'm not familiar with the details of how awesomebar works and the construction of the richlistbox results, so I'd rather not tackle the idea proposed in comment 7 at this point -- I like the idea, but perhaps we should go with what I've got for FF3.

Comment 12

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 304635 [details] [diff] [review]
Patch v.1

Yeah, this is a good second best. Note to self: file a follow-up bug about my super-awesome-idea in the comment above.

Comment 13

[Justin Dolske [:Dolske]]

Attachment: Patch v.2

Will request review after checking tryserver builds.

Comment 14

[Justin Dolske [:Dolske]]

Tryserver builds at: https://build.mozilla.org/tryserver-builds/2008-02-21_02:[e-mail]-bug413497v2/

I think this needs a little more tweaking, though... On Windows, when I type in the urlbar I can get the throbber rapidly flashing (appear, disappear) with each keystroke. Doesn't happen on OS X. 

The solution might be to start a timer in onsearchstart, and only show the throbber after a longer delay. Eg:

Time    Action
0.000s  User types a letter
0.100s  onsearchbegins fires (due to <textbox timeout="100">)
        call setTimeout(400, showThrobber)
0.500s  show throbber

If the search finishes before 0.500, the setTimeout code would just do nothing, and you'd never see the throbber (or thus a flashing).

Comment 15

[Justin Dolske [:Dolske]]

Attachment: Patch v.3

This adds a 500ms delay between when the search starts and when the throbber is first shown. Thus, for fast searches, the throbber is never shown (and the flickering throbber-while-typing problem goes away).

As a general testing note, you can trigger a long awesomebar search by typing in some nonsense (eg "sdfgsdfg") that isn't in your history, which should result an a search of your entire history. Common matches, like "http", will return the max results too quickly. If you're testing in a new profile, though, your history will be mostly empty and no search will be slow enough to result in a throbber.

Comment 16

[Alex Faaborg [:faaborg] (Firefox UX)]

Does this patch use the current throbber?  I should give you a spec for the colors for apng throbbers on OS X, XP, Vista and Linux, I'll try to get that to you soon if you have the "target acquired" apng being generated with canvas.  I'll try to post that soon.

Comment 17

[Asaf Romano (gone)]

What's the <xul:stack> for?

Comment 18

[Asaf Romano (gone)]

And on the next iteration, please rename this object to LocationBarHelpers.

Comment 19

[Justin Dolske [:Dolske]]

(In reply to comment #16)
> Does this patch use the current throbber?

Yes. It's just an image, so that can easily be updated/changed independently.

(In reply to comment #17)
> What's the <xul:stack> for?

It allows for the later possibility of fading the throbber in/out (by adjusting opacity) and is arguably a cleaner implementation for CSS. It also already went through a few review cycles with Dao in bug 402968.

Comment 20

[Asaf Romano (gone)]

Comment on attachment 305059 [details] [diff] [review]
Patch v.3

>Index: browser/base/content/browser.js
>=================================================================== 
>+var AwesomebarSearch = {
>+  timeoutID : null,
>+

Prefix members with an underscore, a space before the colon isn't common in browser/ js code conventions.

>+  begin : function () {

Label the functions.

>+    this.timeoutID = setTimeout(this.delayedBegin, 500);
>+  },
>+
>+  delayedBegin : function () {
>+    this.timeoutID = null;

|this| doesn't point to AwesomebarSearch (btw, comment 18).

>Index: browser/base/content/browser.xul
>===================================================================
>@@ -299,38 +299,36 @@
>                  enablehistory="true"
>                  timeout="100"
>                  maxrows="10"
>                  newlines="stripsurroundingwhitespace"
>                  oninput="URLBarOnInput(event);"
>                  ontextentered="return handleURLBarCommand(param);"
>                  ontextreverted="return handleURLBarRevert();"
>                  pageproxystate="invalid"
>+                 onsearchbegin="AwesomebarSearch.begin();"
>+                 onsearchcomplete="AwesomebarSearch.complete();"

Is it also dispatched as you cancel the search?

Comment 21

[Justin Dolske [:Dolske]]

(In reply to comment #20)

> >+  begin : function () {
> 
> Label the functions.

I'm not sure what you mean. Are you talking about |bar: function FOO_bar ()|? I though that was generally frowned upon, now.

> >+  delayedBegin : function () {
> >+    this.timeoutID = null;
> 
> |this| doesn't point to AwesomebarSearch (btw, comment 18).

Doh. Good catch.

> >+                 onsearchcomplete="AwesomebarSearch.complete();"
> 
> Is it also dispatched as you cancel the search?

Yes.

Comment 22

[Justin Dolske [:Dolske]]

Attachment: Patch, v.4

Comment 23

[Shawn Wilsher :sdwilsh]

(In reply to comment #21)
> I'm not sure what you mean. Are you talking about |bar: function FOO_bar ()|? I
> though that was generally frowned upon, now.
Named functions are useful when trying to use a debugger - I think they are useful for dtrace too, right?

Comment 24

[Asaf Romano (gone)]

Comment on attachment 305268 [details] [diff] [review]
Patch, v.4

The correct way to do this is to pass a scoped function(self) within begin rather than add it to the prototype.

Also, yes, what shawn said.

Comment 25

[Asaf Romano (gone)]

> I'm not sure what you mean. Are you talking about |bar: function FOO_bar ()|? I

I actually meant |bar: function FOO_bar()| ;)

Comment 26

[Justin Dolske [:Dolske]]

(In reply to comment #23)
> Named functions are useful when trying to use a debugger - I think they are
> useful for dtrace too, right?

DTrace is happy without it...

  1 46568553         browser.js                 -> _searchBegin
  1 46568636         browser.js                   -> setTimeout
  1 46568807         browser.js                   <- setTimeout
  1 46568852         browser.js                 <- _searchBegin

I was actually thinking about the performance aspect I had heard Myk mention (see bug 389368), although it's not terribly important here. I personally just don't like doing this because it seems silly to essentially be naming things twice.

Comment 27

[Justin Dolske [:Dolske]]

Attachment: Patch v.5

Comment 28

[Asaf Romano (gone)]

Comment on attachment 305289 [details] [diff] [review]
Patch v.5

ok, r=mano.

Comment 29

[Damon Sicore (:damons)]

Comment on attachment 305289 [details] [diff] [review]
Patch v.5

a1.9+=damons

Comment 30

[Justin Dolske [:Dolske]]

Checking in browser/base/content/browser.css;
  new revision: 1.58; previous revision: 1.57
Checking in browser/base/content/browser.js;
  new revision: 1.976; previous revision: 1.975
Checking in browser/base/content/browser.xul;
  new revision: 1.437; previous revision: 1.436
Checking in browser/themes/gnomestripe/browser/browser.css;
  new revision: 1.192; previous revision: 1.191
Checking in browser/themes/pinstripe/browser/browser.css;
  new revision: 1.128; previous revision: 1.127
Checking in browser/themes/winstripe/browser/browser.css;
  new revision: 1.178; previous revision: 1.177

Comment 31

[Mike Beltzner [:beltzner, not reading bugmail]]

I have this problem where the search is too fast and I don't see the throbber.

Maybe that isn't actually a problem. :)

Comment 32

[Alfred Kayser]

I am not sure if this patch is worth the trouble for the themers, as I am not able to convince Firefox to show the throbber, even if I type a nonsense URL.

Comment 33

[Justin Dolske [:Dolske]]

I don't get the throbber at all (due to the delay described in comment 15) with testing profiles that don't have extensive browsing history. But on my year-old main profile, (which I use daily, with a 45MB places.sqlite), I get the throbber with searches that end up wading through history... For example, while typing "bug" fills up my search results nearly instantly, "zappa" takes about 6 seconds before returning nothing. (And just "zap" gives me a handful of results trickling in over those 6 seconds.)

This will, of course, vary depending on your hardware (cpu/disk), and tweaking browser.urlbar.* prefs can change things too.

Comment 34

[Seth Spitzer]

> I don't get the throbber at all

first, I get the throbber with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008022704 Minefield/3.0b4pre.  [note, that is not my debug build, but a nightly]

second, I really like that there is some sort of progress.   nice work!  I think this bug makes bug #392196 wontfix (or that bug is a dup of this one.)

third, I have a suggestion for a different icon, see bug #420226

Comment 36

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED:

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9pre) Gecko/2008050804 Minefield/3.0pre

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008050804 Minefield/3.0pre

-and-

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050806 Minefield/3.0pre

(I echo dolske's comments in 33, but I am still able to get it to show up when the search is sub-second or so.)

Comment 37

[Stephen Donner [:stephend] Not actively reading bugmail]

Er, not sub-second; I meant >, sigh.