If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Move throbber to where the URL bar site icon (favicon) is.

RESOLVED WONTFIX

Status

()

Firefox
General
P4
enhancement
RESOLVED WONTFIX
10 years ago
10 years ago

People

(Reporter: Dolske, Assigned: Dolske)

Tracking

Trunk
Firefox 3 beta2
Points:
---
Bug Flags:
blocking-firefox3 -
wanted-firefox3 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

(Assignee)

Description

10 years ago
Alex, beltzner, and mconnor designed this, I'm summarizing:

* Remove the throbber from its current location on the toolbar.
* Add throbber to where the favicon is in the URL bar.
* The throbber, like the favicon and the URL, is tied to the tab being shown.
* While the page is loading, you see the throbber, and when it finishes you get the favicon. [Just like how the throbber/icon on tabs currently works.]

Why:

* Brings load activity closer to the UI that affects throbbing (eg Stop, enter a URL, forward/back/reload).
* A throbber here is useful for indicating activity as a Places search runs (as you're typing in the field)
* One less icon in the primary UI (favicon is now overloaded, no standalone throbber)

Known nits:

* The "inactive throbber" state ceases to exist. [More precisely, a favicon is shown when the throbber is inactive.] This is highly desirable for integration consistency on OS X.
* The throbber is no longer global, so if the only tab loading is hidden in tab overflow, there's no at-a-glance activity indication.
* The favicon can load before the page is finished loading, but won't be shown until the page finishes loading and the throbber stops. Stalled loads would be the worst example of this.
* This could be perceived as a flashing effect, where navigating within a site causes icon --> throbber --> icon --> throbber --> icon, etc. We already flash a little between the site icon and generic icon, although it can be subtle.
Flags: blocking-firefox3?
(Assignee)

Updated

10 years ago
Target Milestone: --- → Firefox 3 M10
(Assignee)

Comment 1

10 years ago
Created attachment 287804 [details] [diff] [review]
Patch, work in progress v.1

Rough cut for pinstripe (OS X).

This probably still needs some work on the stack/desk interaction, e.g. if the dragable stuff is expected to work while throbbing. It might make sense to eliminate the deck entirely and toggle CSS visibility instead, although that's a bit more work.

Comment 2

10 years ago
Is this something we can take this late?
>Is this something we can take this late?

We are currently working under the impression that a variety of theme changes have been scheduled to land between beta 1 and beta 2.  Perhaps mconnor or beltzner should chime in so that we are all on the same page regarding if these UI improvements are acceptable at this stage of our development cycle.

Comment 4

10 years ago
I know we scheduled native theming, e.g. the sidebar stuff you've worked on. Perhaps best to explain somewhere outside of bugzilla.

Comment 5

10 years ago
(In reply to comment #0)
> * The throbber is no longer global

As far as I know, it has never been global.
(Assignee)

Comment 6

10 years ago
Sorry, I meant "global to the window".

Comment 7

10 years ago
That's what I meant, too. The throbber is only spinning for the active tab.
>That's what I meant, too. The throbber is only spinning for the active tab.

It looks like this change will clear up that conceptual problem as well (I also actually thought it was global to the window).
(Assignee)

Comment 9

10 years ago
Huh, I don't know what the heck I was thinking!

So, yeah, the toolbar throbber is tab-specific, which means moving it to the URL field is mostly just a positional change. (The slight behavioral change being that the idle state is now the favicon, instead of a static throb frame.)
>Huh, I don't know what the heck I was thinking!

blame the interface, not the user :)

Updated

10 years ago
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P4
Wanted, but not blocking.
Flags: wanted-firefox3+
Flags: blocking-firefox3-
Flags: blocking-firefox3+
I damn hope it will be possible to undo this so-called "enhancement" by means of a theme or an extension.
(Assignee)

Comment 13

10 years ago
Created attachment 291345 [details] [diff] [review]
Patch for review, v.1

I still need to test this on Windows, but it looks right on Linux and OS X.
Attachment #287804 - Attachment is obsolete: true
Attachment #291345 - Flags: ui-review?(faaborg)
Attachment #291345 - Flags: review?(mano)
Comment on attachment 291345 [details] [diff] [review]
Patch for review, v.1

please try gavin/dao/mconnor first.
Attachment #291345 - Flags: review?(mano)
Comment on attachment 291345 [details] [diff] [review]
Patch for review, v.1

switching to beltzner since I lack ui-rness :)
Attachment #291345 - Flags: ui-review?(faaborg) → ui-review?(beltzner)
(Assignee)

Updated

10 years ago
Attachment #291345 - Flags: review?(dao)
Comment on attachment 291345 [details] [diff] [review]
Patch for review, v.1

With this patch, throbberItem.title should be removed from browser.dtd.

>Index: browser/themes/winstripe/browser/browser.css

>+#navigator-throbber {
>+  list-style-image: url("chrome://global/skin/throbber/Throbber-small.gif");
>+  display: none;
>+}
>+
>+#navigator-throbber[busy="true"] {
>+  background-color: -moz-dialog;
>+  display: block;
>+}

The background of the parent node (#identity-box) isn't solid on Windows, and I think it's not going to be solid on Mac.
So that's not going to look properly. But I'll leave that to beltzner.

Even if it would look right, I'd change the CSS rules like this:

#navigator-throbber {
  list-style-image: url("chrome://global/skin/throbber/Throbber-small.gif");
  background-color: -moz-dialog;
}

#navigator-throbber:not([busy="true"]) {
  visibility: hidden;
}
>The background of the parent node (#identity-box) isn't solid on Windows, and I
>think it's not going to be solid on Mac.
>So that's not going to look properly.

The plan is to eventually use a new apng image that works well on the site button's background.
(Assignee)

Comment 18

10 years ago
(In reply to comment #16)

> The background of the parent node (#identity-box) isn't solid on Windows, and I
> think it's not going to be solid on Mac.
> So that's not going to look properly. But I'll leave that to beltzner.

I'm not sure I understand why that's a problem, unless it's just the APNG vs GIF blending Alex mentioned.

> #navigator-throbber:not([busy="true"]) {
>   visibility: hidden;
> }

Unfortunately the animation still seems plays with |visibility: hidden|; there's a bit of CPU usage being used when I did it that way, whereas with |display: none| it was fully idle.

(In reply to comment #18)
> (In reply to comment #16)
> > The background of the parent node (#identity-box) isn't solid on Windows, and I
> > think it's not going to be solid on Mac.
> > So that's not going to look properly. But I'll leave that to beltzner.
> I'm not sure I understand why that's a problem, unless it's just the APNG vs
> GIF blending Alex mentioned.

I guess you've added the -moz-field background color to overlay the favicon, but it will also overlay the appearance of the identity box -- which can be a gradient, for instance.

Otherwise I don't understand the point of background-color: -moz-dialog;.

Note that this is based on reading the patch, I haven't tested it yet.

How about making the throbber the first child of the stack and using his rule:

#navigator-throbber:not([busy="true"]) ,
#navigator-throbber[busy="true"] + #page-proxy-favicon {
  visibility: hidden;
}

> > #navigator-throbber:not([busy="true"]) {
> >   visibility: hidden;
> > }
> Unfortunately the animation still seems plays with |visibility: hidden|;
> there's a bit of CPU usage being used when I did it that way, whereas with
> |display: none| it was fully idle.

OK.
(In reply to comment #19)
> How about making the throbber the first child of the stack and using his rule:
> 
> #navigator-throbber:not([busy="true"]) ,
> #navigator-throbber[busy="true"] + #page-proxy-favicon {
>   visibility: hidden;
> }

err, display: none
(Assignee)

Comment 21

10 years ago
(In reply to comment #19)

> I guess you've added the -moz-field background color to overlay the favicon,
> but it will also overlay the appearance of the identity box -- which can be a
> gradient, for instance.

The identity-box wraps the page-proxy-stack, so the appearance is the same. Indeed, the #identity-box rule is just a few lines down, and I'm setting the background-color as it is.

> How about making the throbber the first child of the stack and using his rule:
> 
> #navigator-throbber:not([busy="true"]) ,
> #navigator-throbber[busy="true"] + #page-proxy-favicon {
>   visibility: hidden;
> }

Yeah, that should work too.

One catch is that Alex wanted to be able to have a fade effect when the throbber starts/stops, and that would require the ordering of the <stack> as I have it. Although, hmmm, I suppose that could be done either way. In any case, it doesn't need to be a design requirement here.
(In reply to comment #21)
> The identity-box wraps the page-proxy-stack, so the appearance is the same.
> Indeed, the #identity-box rule is just a few lines down, and I'm setting the
> background-color as it is.

The background-color for #identity-box is effectless if -moz-appearance:toolbox kicks in, which is the case for most non-Classic Windows themes.

> One catch is that Alex wanted to be able to have a fade effect when the
> throbber starts/stops, and that would require the ordering of the <stack> as I
> have it. Although, hmmm, I suppose that could be done either way. In any case,
> it doesn't need to be a design requirement here.

That should really work either way, I think.
Created attachment 291561 [details]
WinXP screenshot
Comment on attachment 291345 [details] [diff] [review]
Patch for review, v.1

I'm not a peer so this doesn't mean much, but I'm minusing this now because of the background color, even though this is probably more a UI than a code issue, and because you removed throbberItem.title from the markup but not from the dtd.

Otherwise I think this patch is OK. Theoretically, you really want a <deck> rather than a <stack>, but I like how you can hide / display the elements with pure CSS. A stack also allows better fading, in case we want to add that later.
Attachment #291345 - Flags: review?(dao) → review-
A general note on the UI:

(In reply to comment #0)
> Why:
...
> * A throbber here is useful for indicating activity as a Places search runs (as
> you're typing in the field)

This sounds like the user could confuse Places search with content activity.
But in any case, I don't think fetching autocomplete results takes or should ever take long enough to justify an indicator.

> * One less icon in the primary UI (favicon is now overloaded, no standalone
> throbber)

That doesn't seem to really simplify the UI, as the "annoying movement" is happening in a more prominent place.
>This sounds like the user could confuse Places search with content activity.

To clarify, we will only indicate both firefox activity and network activity if we have a two variable throbber.  That will need to be a separate bug, and I'm not sure if we will be able to pull it off in time for this release.  Otherwise, searching the location bar will provide feedback in the first result if nothing has been returned yet.

>I don't think fetching autocomplete results takes or should
>ever take long enough to justify an indicator.

There are a lot of cases where the search may take longer than 50-100ms (especially if the user has increased the size of their history), so we really should be providing some type of feedback to the user.  I've personally found myself wondering if the search was still progressing or if nothing was going to be returned before.  (as a another separate issue we need to say if no results were returned).
(In reply to comment #23)
> Created an attachment (id=291561) [details]
> WinXP screenshot

Note that the background color is also wrong when #identity-box is green for EV certs (currently only in Pinstripe, bug 406321).
(Assignee)

Comment 28

10 years ago
Created attachment 292172 [details] [diff] [review]
Patch for review, v.2

Updated with review comments.

Checked on Windows/Linux/OS X. Tryserver builds with this patch are at https://build.mozilla.org/tryserver-builds/2007-12-06_17:08-jdolske@mozilla.com-1196989652/
Attachment #291345 - Attachment is obsolete: true
Attachment #292172 - Flags: review?(dao)
Attachment #291345 - Flags: ui-review?(beltzner)
(Assignee)

Comment 29

10 years ago
Created attachment 292173 [details]
Screenshots on Vista
Attachment #291561 - Attachment is obsolete: true
(Assignee)

Comment 30

10 years ago
Oh, I should have noted... On the Vista screenshot, the top 3 images are on the current trunk nightly (ie, without this patch). The bottom 4 images are with this patch.
(Assignee)

Updated

10 years ago
Attachment #292172 - Flags: ui-review?(beltzner)
Comment on attachment 292172 [details] [diff] [review]
Patch for review, v.2

>-      <toolbaritem id="throbber-box" title="&throbberItem.title;" align="center" pack="center">

You still didn't remove throbberItem.title from browser.dtd.

> function PageProxyClearIcon ()
> {
>-  if (gProxyDeck.selectedIndex != 0)
>-    gProxyDeck.selectedIndex = 0;
>   if (gProxyFavIcon.hasAttribute("src"))
>     gProxyFavIcon.removeAttribute("src");
> }

While you're at it, please remove the third line.

>+#navigator-throbber:not([busy="true"]),
>+#navigator-throbber[busy="true"] + #page-proxy-favicon {
>+  display: none;
>+}

This should go into browser/base/content/browser.css (which I should have said earlier).

From winstripe and gnomestripe:
> #page-proxy-favicon:not([src]) {
>   list-style-image: url("chrome://global/skin/icons/folder-item.png") !important;
>   -moz-image-region: rect(0px, 16px, 16px, 0px);
> }
>  
> #page-proxy-favicon[pageproxystate="invalid"] {
>   list-style-image: url("chrome://global/skin/icons/folder-item.png") !important;
>   -moz-image-region: rect(32px, 16px, 48px, 0px) !important;
> }

#page-proxy-favicon doesn't have a src attribute if pageproxystate is "invalid", and the !important flags aren't needed.
So instead, use this:

> #page-proxy-favicon:not([src]) {
>   list-style-image: url("chrome://global/skin/icons/folder-item.png");
>   -moz-image-region: rect(0px, 16px, 16px, 0px);
> }
> 
> #page-proxy-favicon[pageproxystate="invalid"] {
>   -moz-image-region: rect(32px, 16px, 48px, 0px);
> }

r=me with that fixed, but I'm still not a peer.
Attachment #292172 - Flags: review?(dao) → review+
(Assignee)

Comment 32

10 years ago
(In reply to comment #31)

> >+#navigator-throbber:not([busy="true"]),
> >+#navigator-throbber[busy="true"] + #page-proxy-favicon {
> >+  display: none;
> >+}
> 
> This should go into browser/base/content/browser.css (which I should have said
> earlier).

What's the rule-of-thumb for putting things into that file, vs the per-theme file? If all the common rules should go there, should a bug be filed on that -- it looks like there's already a lot of duplication. 

Comment 33

10 years ago
(In reply to comment #32)

> What's the rule-of-thumb for putting things into that file, vs the per-theme
> file? If all the common rules should go there, should a bug be filed on that --
> it looks like there's already a lot of duplication.

Basically anything that makes it difficult for the themeing community to make third-party themes for Firefox 3, anything prevents them from themeing certain widgets such as the URL bar, or that raises their collective blood pressure should go into browser/base/content/browser.css.
(In reply to comment #32)
> What's the rule-of-thumb for putting things into that file, vs the per-theme
> file? If all the common rules should go there, should a bug be filed on that --
> it looks like there's already a lot of duplication.

The general rules is that behavioral rules belong in /content, presentational rules in the appropriate /themes files. The rules in /themes aren't used when you install a third-party theme, but the rules in /content are.
(Assignee)

Comment 35

10 years ago
Created attachment 292278 [details] [diff] [review]
Patch for review, v.3

(In reply to comment #31)

> r=me with that fixed, but I'm still not a peer.

My reading of comment #14 was that Mano wanted someone else to make a first-pass review. That's done, so now I'll request the official Mano-Seal-Of-Approval.
Attachment #292172 - Attachment is obsolete: true
Attachment #292278 - Flags: ui-review?(beltzner)
Attachment #292278 - Flags: review?(mano)
Attachment #292172 - Flags: ui-review?(beltzner)
(Assignee)

Comment 36

10 years ago
[Hmm, apparently I can't count. There's no "Patch v.3"]
Attachment #292278 - Attachment description: Patch for review, v.4 → Patch for review, v.3
(Assignee)

Updated

10 years ago
Attachment #292278 - Attachment filename: patch.throbicon.review4 → patch.throbicon.review3
(Assignee)

Comment 37

10 years ago
Created attachment 293056 [details] [diff] [review]
Patch for review, v.4

Same as v.3, with recent merge conflicts fixed.

Mano, can you get to this soon?
Attachment #292278 - Attachment is obsolete: true
Attachment #293056 - Flags: ui-review?(beltzner)
Attachment #293056 - Flags: review?(mano)
Attachment #292278 - Flags: ui-review?(beltzner)
Attachment #292278 - Flags: review?(mano)
Comment on attachment 293056 [details] [diff] [review]
Patch for review, v.4

I find that this actually makes pageloads feel a little slower, as the throbber is more in your face and you can see exactly how long it's taking.

I'd prefer that we switch as soon as the page starts to render, perhaps even doing a nice crossfade from the throbber to the favicon for extra slickness.

(leave the throbber on the tab, though, until the entire page loads)
Attachment #293056 - Flags: ui-review?(beltzner) → ui-review-
Attachment #293056 - Flags: review?(mano)
(Assignee)

Comment 39

10 years ago
So, I'm thinking that the implication of beltzner's comment 38 (which I agree with) is that we not make this change. At least, not as-is.

I did a quick hack to do the throbbing-->not-throbbing transition when the favicon is available. This seemed to help with having the throbber run less, but creates a new problem... The throbber in the URL bar stops throbbing before the throbber on the tab, which is really confusing -- is the page loading, or not? I think any transition point before onload is going to be perceived as randomly stopping the throbber before the page has loaded.

How to proceed?

1) Do nothing, ship with throbber as-is. An option, but I do like the cleaner UI without the dedicated throbber element.

2) Somehow make the throbber-in-urlbar less, well, in-your-face. I don't have a good proposal for this, but a conceptual ideas would be to overlay the favicon with a small throbber (ala Gnome icon emblems), an animated outline, something translucent, etc. 16x16 is a tight space to work in, though.

3) Get rid of the existing throbber, but leave the favicon area in the urlbar alone.

I'm actually leaning in favor of #3. We've already got a throbber on the tab, and bug 392870 seeks to always show the tab bar by default. For cases where we don't show a tabbar, bug 400398 would be a clean solution. [Indeed, #3 is  just implementing the idea in 400398.]
I'd be happy with option #3, fwiw.
I'm leaning towards #3 as well.  This is because I would like to use a slightly different throbber on the site button to indicate that you are searching the awesome bar (the current lack of feedback can be very confusing).  This would be similar to the position of the throbber in spotlight, and we shouldn't mix the meaning of this throbber to cover both searching and loading.
(Assignee)

Comment 42

10 years ago
WONTFIX per above, reopened bug 400398 for option #3.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → WONTFIX

Updated

10 years ago
Duplicate of this bug: 418259
You need to log in before you can comment on or make changes to this bug.