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)
Firefox
Address Bar
Tracking
()
VERIFIED
FIXED
Firefox 3 beta4
People
(Reporter: elreydetodo, Assigned: Dolske)
References
Details
Attachments
(3 files, 4 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
asaf
:
review+
damons
:
approval1.9+
|
Details | Diff | Splinter Review |
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•17 years ago
|
||
Do you have that much history? Does this always happen or only after startup?
Seems similar to bug 413457 (also Linux).
Reporter | ||
Comment 2•17 years ago
|
||
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•17 years ago
|
||
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
Comment 4•17 years ago
|
||
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•17 years ago
|
||
Screen shot of how spotlight provides nice feedback.
Assignee | ||
Comment 6•17 years ago
|
||
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•17 years ago
|
||
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
Comment 8•17 years ago
|
||
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•17 years ago
|
||
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.
Assignee | ||
Comment 10•17 years ago
|
||
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)
Assignee | ||
Comment 11•17 years ago
|
||
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•17 years ago
|
||
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+
Assignee | ||
Comment 13•17 years ago
|
||
Will request review after checking tryserver builds.
Assignee | ||
Comment 14•17 years ago
|
||
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).
Assignee | ||
Comment 15•17 years ago
|
||
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)
Assignee | ||
Updated•17 years ago
|
Comment 16•17 years ago
|
||
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•17 years ago
|
||
What's the <xul:stack> for?
Comment 18•17 years ago
|
||
And on the next iteration, please rename this object to LocationBarHelpers.
Assignee | ||
Comment 19•17 years ago
|
||
(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•17 years ago
|
||
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-
Assignee | ||
Comment 21•17 years ago
|
||
(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.
Assignee | ||
Comment 22•17 years ago
|
||
Attachment #305059 -
Attachment is obsolete: true
Attachment #305268 -
Flags: review?(mano)
Comment 23•17 years ago
|
||
(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•17 years ago
|
||
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-
Comment 25•17 years ago
|
||
> I'm not sure what you mean. Are you talking about |bar: function FOO_bar ()|? I
I actually meant |bar: function FOO_bar()| ;)
Assignee | ||
Comment 26•17 years ago
|
||
(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.
Assignee | ||
Comment 27•17 years ago
|
||
Attachment #305268 -
Attachment is obsolete: true
Attachment #305289 -
Flags: review?(mano)
Comment 28•17 years ago
|
||
Comment on attachment 305289 [details] [diff] [review]
Patch v.5
ok, r=mano.
Attachment #305289 -
Flags: review?(mano) → review+
Assignee | ||
Updated•17 years ago
|
Attachment #305289 -
Flags: approval1.9?
Comment 29•17 years ago
|
||
Comment on attachment 305289 [details] [diff] [review]
Patch v.5
a1.9+=damons
Attachment #305289 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 30•17 years ago
|
||
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
Comment 31•17 years ago
|
||
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•17 years ago
|
||
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.
Assignee | ||
Comment 33•17 years ago
|
||
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•17 years ago
|
||
> 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
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.
Description
•