Closed Bug 413497 Opened 17 years ago Closed 17 years ago

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

Categories

(Firefox :: Address Bar, enhancement)

enhancement
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 3 beta4

People

(Reporter: elreydetodo, Assigned: Dolske)

References

Details

Attachments

(3 files, 4 obsolete files)

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.
Do you have that much history? Does this always happen or only after startup? Seems similar to bug 413457 (also Linux).
Depends on: 413457
Keywords: perf
Version: unspecified → Trunk
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?
Yes, we definitely need to provide feedback to the user that the search is still commencing. Nominating for blocking.
Flags: blocking-firefox3?
Summary: awesome bar should tell you it's still searching → Awesome bar should use a throbber on the site button to provide feedback that search is in progress
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.
Attached image spotlight throbber screen shot (deleted) —
Screen shot of how spotlight provides nice feedback.
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.
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 .... | '-------------------------------------------------'
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: wanted-firefox3+
Flags: blocking-firefox3?
Flags: blocking-firefox3-
Summary: Awesome bar should use a throbber on the site button to provide feedback that search is in progress → Awesome bar should use a throbber to provide feedback that search is in progress
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.
Attached image "target acquired" throbber idea (deleted) —
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.
Depends on: 418712
Attached patch Patch v.1 (obsolete) (deleted) — Splinter Review
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).
Attachment #304635 - Flags: ui-review?(beltzner)
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 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.
Attachment #304635 - Flags: ui-review?(beltzner) → ui-review+
Attached patch Patch v.2 (obsolete) (deleted) — Splinter Review
Will request review after checking tryserver builds.
Assignee: nobody → dolske
Attachment #304635 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Tryserver builds at: https://build.mozilla.org/tryserver-builds/2008-02-21_02:32-jdolske@mozilla.com-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).
Attached patch Patch v.3 (obsolete) (deleted) — Splinter Review
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.
Attachment #304677 - Attachment is obsolete: true
Attachment #305059 - Flags: review?(mano)
Keywords: perf
OS: Linux → All
Hardware: PC → All
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.
What's the <xul:stack> for?
And on the next iteration, please rename this object to LocationBarHelpers.
(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 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?
Attachment #305059 - Flags: review?(mano) → review-
(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.
Attached patch Patch, v.4 (obsolete) (deleted) — Splinter Review
Attachment #305059 - Attachment is obsolete: true
Attachment #305268 - Flags: review?(mano)
(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 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.
Attachment #305268 - Flags: review?(mano) → review-
> I'm not sure what you mean. Are you talking about |bar: function FOO_bar ()|? I I actually meant |bar: function FOO_bar()| ;)
(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.
Attached patch Patch v.5 (deleted) — Splinter Review
Attachment #305268 - Attachment is obsolete: true
Attachment #305289 - Flags: review?(mano)
Comment on attachment 305289 [details] [diff] [review] Patch v.5 ok, r=mano.
Attachment #305289 - Flags: review?(mano) → review+
Attachment #305289 - Flags: approval1.9?
Comment on attachment 305289 [details] [diff] [review] Patch v.5 a1.9+=damons
Attachment #305289 - Flags: approval1.9? → approval1.9+
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
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 beta4
I have this problem where the search is too fast and I don't see the throbber. Maybe that isn't actually a problem. :)
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.
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.
Blocks: 420226
> 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
Depends on: 426723
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.)
Status: RESOLVED → VERIFIED
Er, not sub-second; I meant >, sigh.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: