return to using non deterministic throbbers (remove progress line / bar implementation)

VERIFIED FIXED in Firefox 4.0b7

Status

()

Firefox
Tabbed Browser
VERIFIED FIXED
7 years ago
3 years ago

People

(Reporter: beltzner, Assigned: Margaret)

Tracking

(Depends on: 3 bugs)

Trunk
Firefox 4.0b7
Points:
---
Dependency tree / graph
Bug Flags:
in-litmus ?

Firefox Tracking Flags

(blocking2.0 beta7+)

Details

(Whiteboard: THIS BUG IS CLOSED, NO MORE COMMENTS, PLEASE!)

Attachments

(2 attachments, 4 obsolete attachments)

Created attachment 481900 [details]
throbber sketch (excuse the bad artwork)

The UX team met to discuss the progress lines added in bug 544818 to all tabs as part of the new theme. We agreed that while the design has potential and fits the theme well, the problems that we're experiencing in the implementation have outweighed the potential benefits.

The main problems were:
 - (design) different location for background and foreground tabs
 - (design) hard to notice / too small
 - (design) removed important feedback about connection state
 - (implementation) no way to truthfully relay overall progress
 - (implementation) lack of immediate feedback
 - (implementation) wrong color
 - (implementation) 30% CPU performance hit

While we may choose to revisit the design in the future, we need a replacement design which meets the following goals, as decided by the UX team:

* need to have some feedback immediately after click
* needs to always be animated (OSX/Win7 style, brightness change and pattern change)
* needs to illustrate at least the difference between "seeking connection" and "loading" states

Some difference of opinion exists about whether or not a third state (establishing connection, which can take some time) should be represented.

Attached is a sketch of a throbber design that does that. This bug is to track the removal of the progress line indicator code and replace it with the throbbers.
Note: this will not restore the verbose text information that used to display in the status bar; that will be dealt with separately, but I'll reference it once the bug is filed.

Updated

7 years ago
Flags: in-litmus?(abillings)
cc'ing Dao, who's on review for a lot of the progress line bugs, which should be switched to INVALID or WONTFIX in light of this one.
(Assignee)

Comment 3

7 years ago
Created attachment 481903 [details] [diff] [review]
patch

This patch has two states for the throbber, one for before we start making any progress and one after we start making progress. I can add another state, but that will require more changes to the onStateChange listener.

Also, this patch still needs the correct icons. I just inserted a random toolbar PNG to use as a placeholder.
Attachment #481903 - Flags: review?(dao)
> - (design) different location for background and foreground tabs

yeah, this was definitely an issue with the progress lines.

> - (design) hard to notice / too small
> - (design) removed important feedback about connection state

These were really more implementation than design. Visibility is mostly based on a constant change of state.  For instance the old throbber was small, and appeared as dark grey and light grey/tan.  The only thing that drew attention to it was the movement.  Also the progress lines had work in progress for showing two states, so showing feedback about the connection was also really more of an implementation issue than a design flaw.

Anyway, on to working on the new design:

prior to getting the page title, when we are still waiting for DNS to resolve and the throbber is in state 1, can we set the tab text to "Connecting..."  This provides a clear textural description of what the first state of the throbber is, and we don't really have any other text to display there yet.
(In reply to comment #4)
> to it was the movement.  Also the progress lines had work in progress for
> showing two states, so showing feedback about the connection was also really
> more of an implementation issue than a design flaw.

Here I meant getting rid of the "Connecting..." text at the bottom, specifically, which may have been going to far. We failed to fully prototype it and evaluate the experience in terms of perception of performance. We learn through these sorts of failures, though, so that's OK :)

> prior to getting the page title, when we are still waiting for DNS to resolve
> and the throbber is in state 1, can we set the tab text to "Connecting..." 
> This provides a clear textural description of what the first state of the
> throbber is, and we don't really have any other text to display there yet.

I think that'd be a good change, yeah. I'd suggest we display the new URL in the location bar immediately (reverting if the page load is cancelled, natch) and put the "Connecting..." over to the right where the link preview went. This puts it, conveniently, right beside the "Stop" button.

Margaret: let us know if you want this in a separate bug, as I alluded to in an earlier comment.

Comment 6

7 years ago
Your artwork is actually quite good, honestly. ;)

To mention a point in the mockup which I think is an inspired idea: the usage of rotation direction as an indicator. Counter-clockwise to start then clockwise once a connection is beginning. This is a very simple addition that adds a lot of feedback.

To summarize attachment 481900 [details] in the bug itself:
1: start event -> CCW rotation throbber
2: DNS + request -> CW double rotation throbber (small inside large)
3: receive data -> CW wide blue progress indicator circle
4: completion -> favicon
I think we currently say "Loading..." in background tabs that haven't resolved DNS yet.  This doesn't directly explain if we have received any information (does it mean loading in that Firefox is taking awhile to display it?), so we should probably change it to "Connecting..." for internal consistency.

Comment 8

7 years ago
This bug will also fix the problems with loading indication for app tabs.

Comment 9

7 years ago
Personally, I really like the progress lines and will be very sad to see them go.  Obviously the performance is an issue, but as you said, this is an implementation detail.  My biggest problem with the lines is the differing location between foreground and background tabs, but this clearly isn't a fundamental limitation of the design.

All in all, I thought the progress lines were very nice and a very distinctive feature in terms of the overall Firefox UX.  Other browsers (most notably, Chrome) are going with the favicon-replaced-by-throbber concept.  Chrome even has the proposed counter-clockwise / clockwise state representation.  However, no one does tab-level progress lines.  I really wish that we could keep them.  Smooth out the performance problems and fix the minor consistency niggles, but don't be so quick to completely abandon the feature.

Updated

7 years ago
blocking2.0: --- → ?
Beta 8 at least, but if we can get it for beta 7, bully for us.

Daniel: the progress lines were also, ultimately, lies. We don't know how far along the pageload is, and there is no way for us to do so.

(All: please remember that factual discussion is fine, evangelism is not.)
blocking2.0: ? → beta8+

Comment 11

7 years ago
(In reply to comment #10)
> Daniel: the progress lines were also, ultimately, lies. We don't know how far
> along the pageload is, and there is no way for us to do so.

This is a fair point.  It was a nice placebo though, and certainly a clever way of rendering what information we did have.  I wish we could unify the approaches somehow.  I really don't care for the fact that we lose the favicon during load (making it slightly harder to identify tabs, particularly app tabs).
(In reply to comment #11)
> This is a fair point.  It was a nice placebo though, and certainly a clever way
> of rendering what information we did have.  I wish we could unify the
> approaches somehow.  I really don't care for the fact that we lose the favicon
> during load (making it slightly harder to identify tabs, particularly app
> tabs).

We're off topic for this bug, so let's have this be the last comment pining for the short-lived progress lines: we didn't have information. We had *no* information. We were just drawing a line in ever-smaller increments until the load stopped.
(Assignee)

Comment 13

7 years ago
(In reply to comment #5)
> (In reply to comment #4)
> > prior to getting the page title, when we are still waiting for DNS to resolve
> > and the throbber is in state 1, can we set the tab text to "Connecting..." 
> > This provides a clear textural description of what the first state of the
> > throbber is, and we don't really have any other text to display there yet.
> 
> I think that'd be a good change, yeah. I'd suggest we display the new URL in
> the location bar immediately (reverting if the page load is cancelled, natch)
> and put the "Connecting..." over to the right where the link preview went. This
> puts it, conveniently, right beside the "Stop" button.
> 
> Margaret: let us know if you want this in a separate bug, as I alluded to in an
> earlier comment.

Yeah, this would probably be better in a separate bug, since that will make it easier to land these throbbers as soon as they're ready.

(In reply to comment #6)
> To summarize attachment 481900 [details] in the bug itself:
> 1: start event -> CCW rotation throbber
> 2: DNS + request -> CW double rotation throbber (small inside large)
> 3: receive data -> CW wide blue progress indicator circle
> 4: completion -> favicon

I'm not sure how to get a signal for state 2. We're using nsIWebProgressListener (http://mxr.mozilla.org/mozilla-central/source/uriloader/base/nsIWebProgressListener.idl#55) to listen for start and progress events, but it doesn't look like that will give us the information we need for state 2.

Comment 14

7 years ago
(In reply to comment #13)
> (In reply to comment #6)
> > To summarize attachment 481900 [details] [details] in the bug itself:
> > 1: start event -> CCW rotation throbber
> > 2: DNS + request -> CW double rotation throbber (small inside large)
> > 3: receive data -> CW wide blue progress indicator circle
> > 4: completion -> favicon
> 
> I'm not sure how to get a signal for state 2. We're using
> nsIWebProgressListener
> (http://mxr.mozilla.org/mozilla-central/source/uriloader/base/nsIWebProgressListener.idl#55)
> to listen for start and progress events, but it doesn't look like that will
> give us the information we need for state 2.

Isn't state 2 simply STATE_START?

http://mxr.mozilla.org/mozilla-central/source/uriloader/base/nsIWebProgressListener.idl#69
> STATE_START
>   This flag indicates the start of a request.  This flag is set when a
>   request is initiated.  The request is complete when onStateChange is
>   called for the same request with the STATE_STOP flag set.

If I'm reading that correctly, STATE_START is on initial send of the request which is necessarily after the DNS request.

Comment 15

7 years ago
And then state 3 is STATE_TRANSFERRING and state 4 is STATE_STOP. (assuming the documentation matches stated effect exactly as we want it here)

Also, STATE_REDIRECTING would necessarily imply jumping back to state 1 again. Thus, for a set of redirects it would naturally between states 1 & 2 until it begins loading the end destination and going on to state 3 & 4.
What states do you want?  We control the horizontal and vertical here, and should be sending the notifications that are of interest to the consumers.  Please file bugs on notification changes that you desire, and -- subject to likely being after the extension API-freeze when we do the work -- your wishes have a very high chance of being platform's command.
(Assignee)

Comment 17

7 years ago
According to my testing, STATE_START happens immediately after a request is sent, regardless of whether or not the browser hears anything back, so the throbber wouldn't be spending any time in state 1. Therefore, if we want to differentiate between states 1 and 2 we will need to find some other event to listen for.

Comment 18

7 years ago
(In reply to comment #17)
> According to my testing, STATE_START happens immediately after a request is
> sent, regardless of whether or not the browser hears anything back, so the
> throbber wouldn't be spending any time in state 1.

You would see state 1 for the DNS request before the HTTP request, which would be short or long depending on whether the request goes through quickly.

Comment 19

7 years ago
(In reply to comment #10)
> Beta 8 at least, but if we can get it for beta 7, bully for us.

If this can't make beta 7, then the changesets for all progress lines should probably be backed out for beta 7 reverting things to beta 6 style progress indicators, otherwise it's essentially requesting useless feedback on a feature that is already being replaced. It'd confuse beta testers and those who'd write reviews of the "feature freeze" beta.
Comment on attachment 481903 [details] [diff] [review]
patch

>-  onProgressChange: function (aWebProgress, aRequest,
>-                              aCurSelfProgress, aMaxSelfProgress,
>-                              aCurTotalProgress, aMaxTotalProgress) {
>-    // Check this._busyUI to be safe, because we don't want to update
>-    // the progress meter when restoring a page from bfcache.
>-    if (aMaxTotalProgress > 0 && this._busyUI) {
>-      // This is highly optimized.  Don't touch this code unless
>-      // you are intimately familiar with the cost of setting
>-      // attrs on XUL elements. -- hyatt
>-      let percentage = (aCurTotalProgress * 100) / aMaxTotalProgress;
>-      this.statusMeter.value = percentage;
>-    }
>-  },
>-
>-  onProgressChange64: function (aWebProgress, aRequest,
>-                                aCurSelfProgress, aMaxSelfProgress,
>-                                aCurTotalProgress, aMaxTotalProgress) {
>-    return this.onProgressChange(aWebProgress, aRequest,
>-      aCurSelfProgress, aMaxSelfProgress, aCurTotalProgress,
>-      aMaxTotalProgress);
>-  },

I think you need to implement these methods in case mTabbedMode is false in gBrowser.addProgressListener.

>+              if (this.mTotalProgress && !this.mTab.hasAttribute("progress"))
>+                this.mTab.setAttribute("progress", true);

The hasAttribute call doesn't really make this more efficient, you can just set the attribute regardless.

nit: s/true/"true"/

>+      <method name="updateIcon">
>+         <parameter name="aTab"/>
>+         <body>
>+            <![CDATA[
>+               var browser = this.getBrowserForTab(aTab);
>+               if (!aTab.hasAttribute("busy") && browser.mIconURL)
>+                  aTab.setAttribute("image", browser.mIconURL);

nit: indentation is off

>+               else
>+                  aTab.removeAttribute("image");
>+               this._tabAttrModified(aTab);

_tabAttrModified shouldn't be called if the attribute hasn't changed.

Instead re-introducing updateIcon, can we add a separate image and toggle it depending on its state?

e.g:

.tab-throbber:not([busy]),
.tab-throbber[busy] + .tab-icon-image {
  display: none;
}
Attachment #481903 - Flags: review?(dao) → review-

Comment 21

7 years ago
This is not a good idea for Mozilla to do this. The Progress Lines in the URL bar gave Firefox a unique look that other browsers do not. I always thought that Mozilla was sort of a leader in new ideas and not just a tag along. Going backwards is not the way to compete in this market.

Comment 22

7 years ago
(In reply to comment #21)
> This is not a good idea for Mozilla to do this. The Progress Lines in the URL
> bar gave Firefox a unique look that other browsers do not. I always thought
> that Mozilla was sort of a leader in new ideas and not just a tag along. Going
> backwards is not the way to compete in this market.

Please read comment 0

Updated

7 years ago
Duplicate of this bug: 603068
Regarding high CPU usage, I asked this many time, why don't use HW acceleration for it?

The problem throbbers bring to table is that it hides favicon and I'm looking ta favicons on tabs, not on title. It's easier and faster.

Comment 25

7 years ago
(In reply to comment #24)
> Regarding high CPU usage, I asked this many time, why don't use HW acceleration
> for it?

a) The 30% CPU performance hit happened as well with HA enabled
b) Not everybody has HA enabled (lots of drivers blacklisted)
c) This would still not solve all issues in comment 0

Comment 26

7 years ago
Trobbers - OK
But please, make it like a piechart, like before in Fx4 nightlies

Comment 27

7 years ago
Maybe we could have the progress lines if hardware acceleration is enabled?
Piecharts OK.
But not grey or black please! Blue as the above attachment or any other color but not black please!
The Artwork sounds like a good variation of the current Activity Indicator widget. I like it.

Comment 30

7 years ago
If the progress-line is to thin - why not go the Fission-way and show the progress-bar as a background throughout the whole url-bar?

Benefits:
- easy to recognize
- easy in processing (i.e. no 30% performance-hit)
- easy to implement (right now, only a couple of CSS-lines is needed)
- easy to distinguish (different connection states = different coloring)

Regarding the background-tabs' progress-line, I agree that it's hard to recognize and a good throbber-representation on it might be better.

JM2C..

Comment 31

7 years ago
If we're still going ahead with this, then we could just have an add-on for bringing back the progress lines.

Comment 32

7 years ago
(In reply to comment #31)
> If we're still going ahead with this, then we could just have an add-on for
> bringing back the progress lines.

There has been https://addons.mozilla.org/en-US/firefox/addon/14644/ for 3.6, which could be brought forward...

Comment 33

7 years ago
Though, it wouldn't bring back the progress-bar below (or, if doing it the Fission-way, the url-bar's background), right?

Comment 34

7 years ago
(In reply to comment #30)
> If the progress-line is to thin - why not go the Fission-way and show the
> progress-bar as a background throughout the whole url-bar?
> 
> Benefits:
> - easy to recognize
> - easy in processing (i.e. no 30% performance-hit)
> - easy to implement (right now, only a couple of CSS-lines is needed)
> - easy to distinguish (different connection states = different coloring)
> 
> Regarding the background-tabs' progress-line, I agree that it's hard to
> recognize and a good throbber-representation on it might be better.
> 
> JM2C..

Also it could co-exist with the pie chart/throbber.

Comment 35

7 years ago
(In reply to comment #0)
Is there a usergroup thread for the discussion of the swap of the PLs?

Anyway, here are my thoughts

I am absolutely disappointed about the removal of the progress lines. Those worked perfectly for me. At a glance I had all the information I wanted across all tabs.

>    - (design) different location for background and foreground tabs
What is so wrong with that? It also helped people moaning about the differential between foreground/background tabs differentiate.

>    - (design) hard to notice / too small
I'm not sure how. But this could have been solved with using the colour suggested in the mockups (green) rather than the blue that appeared in the implementation.

>    - (design) removed important feedback about connection state
This could've been fixed easily by flashing a full progress line while attempting to connect and then having the moving progress line when loading. If the connection fails you could flash it red.

>    - (implementation) no way to truthfully relay overall progress
I'm not sure I get this. The whole point of the progress line is that when it gets to the end it's done.

>    - (implementation) lack of immediate feedback
Could be fixed by my suggestions for "(design) removed important feedback about connection state"

>    - (implementation) wrong color
That's a CSS value. The original green was perfect, I'm not sure how we ended up on blue.

>    - (implementation) 30% CPU performance hit
I remember seeing a discussion whereby it was said the original pie chart used more CPU cycles, this was originally viewed as a win. There seems to be this common idea floating around that most of the CPU performance hit was caused by the gradient, which I'd previously complained about stating it skewed the ability to ascertain progress and as such should've been removed.

Of course, if we can't get the CPU hit under control, this is the only sensible option, but I'd hope we could invest a little more time into that and getting the progress lines right before we gave up on them, even if only temporarily.

Comment 36

7 years ago
For the record, this bug will solve (or not) Bug 573272 - activity indicator (throbber/spinner) doesn't look native in GNOME with firefox-3.7a
(Assignee)

Comment 37

7 years ago
(In reply to comment #36)
> For the record, this bug will solve (or not) Bug 573272 - activity indicator
> (throbber/spinner) doesn't look native in GNOME with firefox-3.7a

The toolbar throbber is controlled by different code than the tab throbbers, but I think it's a good idea to consider making their appearance the same, probably in a follow-up bug.
(Assignee)

Comment 38

7 years ago
(In reply to comment #20)
> Comment on attachment 481903 [details] [diff] [review]
> patch
> >+               else
> >+                  aTab.removeAttribute("image");
> >+               this._tabAttrModified(aTab);
> 
> _tabAttrModified shouldn't be called if the attribute hasn't changed.

I just copied the old updateIcon method (http://hg.mozilla.org/mozilla-central/file/eec9a82ad740/browser/base/content/tabbrowser.xml#l622). Is there an easy way to tell if the attribute actually changed?

> Instead re-introducing updateIcon, can we add a separate image and toggle it
> depending on its state?
> 
> e.g:
> 
> .tab-throbber:not([busy]),
> .tab-throbber[busy] + .tab-icon-image {
>   display: none;
> }

I'll try working on this now, but maybe it would be best to just revert to what we had before for now to try to land this for beta 7. I think it would be confusing to beta testers (and not really helpful to us) to have the progress lines go into one beta release when we know we want to take them out for the next.
(In reply to comment #38)
> (In reply to comment #20)
> > Comment on attachment 481903 [details] [diff] [review] [details]
> > patch
> > >+               else
> > >+                  aTab.removeAttribute("image");
> > >+               this._tabAttrModified(aTab);
> > 
> > _tabAttrModified shouldn't be called if the attribute hasn't changed.
> 
> I just copied the old updateIcon method
> (http://hg.mozilla.org/mozilla-central/file/eec9a82ad740/browser/base/content/tabbrowser.xml#l622).
> Is there an easy way to tell if the attribute actually changed?

You can call getAttribute before changing it.

> > Instead re-introducing updateIcon, can we add a separate image and toggle it
> > depending on its state?
> > 
> > e.g:
> > 
> > .tab-throbber:not([busy]),
> > .tab-throbber[busy] + .tab-icon-image {
> >   display: none;
> > }
> 
> I'll try working on this now, but maybe it would be best to just revert to what
> we had before for now to try to land this for beta 7.

For that we could just straightly back out the progress line and subsequent patches on the relbranch, couldn't we?

Comment 40

7 years ago
> > I'll try working on this now, but maybe it would be best to just revert to what
> > we had before for now to try to land this for beta 7.
> 
> For that we could just straightly back out the progress line and subsequent
> patches on the relbranch, couldn't we?

I would suggest to not do this. Give beta-testers a chance to test it - maybe they like it and if afterwards removed, they might join the request for improving instead of removing the progress-lines.
(Assignee)

Comment 41

7 years ago
Created attachment 482303 [details] [diff] [review]
patch v2

Addressed review comments, including getting rid of updateIcon.
Attachment #481903 - Attachment is obsolete: true
Attachment #482303 - Flags: review?(dao)
Comment on attachment 482303 [details] [diff] [review]
patch v2

>+.tab-throbber {
>+  list-style-image: url("chrome://browser/skin/tabbrowser/progress.png") !important;
>+  -moz-image-region: rect(0 16px 16px 0);
> }
>+
>+.tab-throbber[progress] {
>+  -moz-image-region: rect(0 32px 16px 16px) !important;
>+}

Both 'important' flags shouldn't be needed.

Why is this in content CSS? Just because it's temporary?

>+          <xul:image xbl:inherits="validate,fadein,pinned,busy,progress,selected"
>+                     class="tab-throbber"
>+                     role="presentation"/>

I don't think you want to inherit 'validate' here.
(Assignee)

Comment 43

7 years ago
(In reply to comment #42)
> Why is this in content CSS? Just because it's temporary?

I put it in content because it's the same for all themes, but it may not always be that way, so I'll move it to theme CSS. Also, the image is in the theme directory, so it makes sense for the CSS rules to live there as well.
(Assignee)

Comment 44

7 years ago
Created attachment 482317 [details] [diff] [review]
patch v3
Attachment #482303 - Attachment is obsolete: true
Attachment #482317 - Flags: review?(dao)
Attachment #482303 - Flags: review?(dao)
(Assignee)

Updated

7 years ago
Attachment #482317 - Attachment is patch: true
Attachment #482317 - Attachment mime type: application/octet-stream → text/plain

Updated

7 years ago
Attachment #482317 - Flags: review?(dao) → review+
(Assignee)

Comment 45

7 years ago
Created attachment 482375 [details] [diff] [review]
patch v4

Stephen gave me the APNGs, and I had to change the CSS to use separate images for the different states.
Attachment #482317 - Attachment is obsolete: true
Attachment #482375 - Flags: review?(dao)
Is there an issue with leaving the "urlbar-progress-container" in the binding?

I only ask because I want to provide a urlbar progress line via my Status-4-Evar extension, but I'd like to avoid overriding the urlbar binding... Unless there's a way to overlay XBL content (or modify it via JS) that I don't know about, that seems like my only avenue with this patch.
Apparently bug 578028 introduced urlbar-contents. Can you remove this as well?
Comment on attachment 482375 [details] [diff] [review]
patch v4

This lacks connecting and loading.png.
Attachment #482375 - Flags: review?(dao) → review-
(Assignee)

Comment 49

7 years ago
(In reply to comment #47)
> Apparently bug 578028 introduced urlbar-contents. Can you remove this as well?

I can do that, but I would need to leave it if I leave
urlbar-progress-container, as requested in comment 46. What do you think?
(In reply to comment #46)
> Is there an issue with leaving the "urlbar-progress-container" in the binding?

We don't use it, so it doesn't make sense for us to maintain it. We wouldn't notice when we break it.

Comment 51

7 years ago
Speaking as an extension developer, I'd prefer Firefox do its thing and not leave any unused widgets around on the off chance that someone might extend it. We're perfectly capable of inserting in our own widgets if the need arises.
(Assignee)

Comment 52

7 years ago
Created attachment 482393 [details] [diff] [review]
patch v5

Added the images and removed urlbar-contents.
Attachment #482375 - Attachment is obsolete: true
Attachment #482393 - Flags: review?(dao)
Comment on attachment 482393 [details] [diff] [review]
patch v5

I hope the APNGs don't have to many fps, regressing bug 437829.
Attachment #482393 - Flags: review?(dao) → review+
(The impact of a too high fps rate may or may not be worse than bug 602126.)
(Assignee)

Comment 55

7 years ago
:( We can never win! Well, Stephen still has to do theme work on the throbbers for different platforms, so we can modify them if they're problematic.
(In reply to comment #55)
> :( We can never win! Well, Stephen still has to do theme work on the throbbers
> for different platforms, so we can modify them if they're problematic.

I think the ones I gave you can be streamlined more.f

Comment 57

7 years ago
(In reply to comment #54)
> (The impact of a too high fps rate may or may not be worse than bug 602126.)

Spin a try-server build so people can test it before landing?

Comment 58

7 years ago
(In reply to comment #57)
> Spin a try-server build so people can test it before landing?

Not worth it; just land it on Trunk and b7 and tweak after the fact.

Comment 59

7 years ago
Comment on attachment 482393 [details] [diff] [review]
patch v5

(In reply to comment #47)
> Apparently bug 578028 introduced urlbar-contents. Can you remove this as well?

This patch removes the urlbar-contents classed vbox itself but the CSS is still there. I think these CSS bits need excising too.

Would it be easier/cleaner to back out the progress lines' various changesets first then apply a new more concise patch on top of that?
(In reply to comment #50)
> We don't use it, so it doesn't make sense for us to maintain it. We wouldn't
> notice when we break it.

(In reply to comment #51)
> Speaking as an extension developer, I'd prefer Firefox do its thing and not
> leave any unused widgets around on the off chance that someone might extend it.
> We're perfectly capable of inserting in our own widgets if the need arises.


I'm aware that hanging on to unused elements is generally bad practice. But, unless I'm missing something, XBL isn't really extension friendly, from both a changing Fx implementation and a cross-extension compatibility standpoint. Overriding/replacing the entire content binding of a major UI element, just to add a single element (and associated wrappers), is a pretty fugley solution.
(Assignee)

Comment 61

7 years ago
(In reply to comment #59)
> Comment on attachment 482393 [details] [diff] [review]
> patch v5
> 
> (In reply to comment #47)
> > Apparently bug 578028 introduced urlbar-contents. Can you remove this as well?
> 
> This patch removes the urlbar-contents classed vbox itself but the CSS is still
> there. I think these CSS bits need excising too.

Oops, good catch!

> Would it be easier/cleaner to back out the progress lines' various changesets
> first then apply a new more concise patch on top of that?

I think at this point, I can just do that last cleanup, since this patch is so close to being complete.
(Assignee)

Updated

7 years ago
Blocks: 603541
(Assignee)

Comment 62

7 years ago
http://hg.mozilla.org/mozilla-central/rev/42270894db65

I filed bug 603541 to create theme-specific throbber images.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Updated

7 years ago
Target Milestone: --- → Firefox 4.0b8

Comment 63

7 years ago
it that possible add a symbol after DOM is loaded?

Comment 64

7 years ago
(In reply to comment #61)
> > This patch removes the urlbar-contents classed vbox itself but the CSS is still
> > there. I think these CSS bits need excising too.
> 
> Oops, good catch!
> 
> > Would it be easier/cleaner to back out the progress lines' various changesets
> > first then apply a new more concise patch on top of that?
> 
> I think at this point, I can just do that last cleanup, since this patch is so
> close to being complete.

Fair enough. Looks like you also got the urlbar-history-dropmarker class that was introduced too when you committed (comment 62).

Comment 65

7 years ago
Testing this out in an hourly build it looks good, but sometimes the throbber stutters a little bit. Is there any way to force it to animate more smoothly even if (or especially if) Firefox is busy loading the page?
Depends on: 603594

Comment 66

7 years ago
Just seeing this in the nightly after updating Minefield (mac) and think the grey throbber looks fantastic! However the grey version swaps to a bright blue throbber about halfway through a load. I assume this is just a nightly bug as I'm not sure why we'd want to change throbbers mid-load, but if not, I'd like to recommend sticking solely with the grey version for the mac theme. Thanks!
The different color shows the different state of the loading.

gray: DNS lookup
blue: content loading
Please keep it gray and blue, at least for Windows! Only gray is so dull and boring now that we won't see the nice blue progress line any more!

Comment 69

7 years ago
(In reply to comment #67)
> The different color shows the different state of the loading.
> 
> gray: DNS lookup
> blue: content loading

Ah, I see. To me, this seems a little jarring and rather distracting, especially since they spin in opposite directions. I'd assume most users view page loading holistically and it seems unnecessarily noisy to differentiate the two states. Considering how quickly most DNS lookups will occur, this may appear more like a visual glitch to the average user than discernibly useful information. Just throwing my two cents out there, as you guys are doing a fantastic job and I certainly don't mean to step on anyone's toes.

Comment 70

7 years ago
(In reply to comment #69)
> (In reply to comment #67)
> > The different color shows the different state of the loading.
> > 
> > gray: DNS lookup
> > blue: content loading
> 
> Ah, I see. To me, this seems a little jarring and rather distracting,
> especially since they spin in opposite directions. I'd assume most users view
> page loading holistically and it seems unnecessarily noisy to differentiate the
> two states. Considering how quickly most DNS lookups will occur, this may
> appear more like a visual glitch to the average user than discernibly useful
> information. Just throwing my two cents out there, as you guys are doing a
> fantastic job and I certainly don't mean to step on anyone's toes.

Folks, this is just the same styling as in Google Chrome, and they've been doing just fine. No point in discussing colors in a "Resolved fixed" bug. File new bugs if you must.
To me it looks awesome!!!
Not jarring, nor distracting, nor noisy, nor like a visual glitch at all, not only awesome but a very nice way to replace the progress lines!!!
Great, now we can all spam Bugzilla how great the new throbber is...
All: please remember that this bug record is meant to be a tracking of the *implementation* of the design work. Please reserve your comments to the following topics:

 - specifics of the implementation (optimizations, interpretation of state, etc)
 - performance or perception thereof, along with suggestions of how to improve/react
 - specific colour or pixel tweaks needed to meet the design specification

Evangelism about the design approach, or the one it replaced, is not welcome here. Neither are suggestions of how to tweak the specific visual design, which should instead be taken to mozilla.dev.apps.firefox / dev-apps-firefox@lists.mozilla.org

Comment 74

7 years ago
(In reply to comment #67)
> The different color shows the different state of the loading.
> 
> gray: DNS lookup
> blue: content loading

gray is used for the 'connecting state', which is more than just the DNS lookup.
blocking2.0: beta8+ → beta7+

Updated

7 years ago
Depends on: 603658
(Assignee)

Updated

7 years ago
Blocks: 603659

Comment 75

7 years ago
Can anybody make addon that returns progress lines (for each tab and under address bar). So many work to /dev/null...

Comment 76

7 years ago
(In reply to comment #75)
> Can anybody make addon that returns progress lines (for each tab and under
> address bar). So many work to /dev/null...

Big +1

(In reply to comment #73)
>  - performance or perception thereof, along with suggestions of how to
> improve/react

I think the blue arrow/circle should fill-in instead of spin to give a more accurate sense of "Loading...", I believe that's what I miss from progress lines now that they're gone.

Updated

7 years ago
Depends on: 600081

Updated

7 years ago
No longer depends on: 603658

Updated

7 years ago
Blocks: 603717

Comment 77

7 years ago
the idea should be easy, if it has three stages, its really hard to explain and just may not be necessary.

I think the early stage pie chart is easier to understand.

Comment 78

7 years ago
Is this ready to land on the b7 relbranch?
(In reply to comment #78)
> Is this ready to land on the b7 relbranch?

Didn't it land this morning?
http://hg.mozilla.org/mozilla-central/rev/409f93411380
(Assignee)

Comment 80

7 years ago
(In reply to comment #79)
> (In reply to comment #78)
> > Is this ready to land on the b7 relbranch?
> 
> Didn't it land this morning?
> http://hg.mozilla.org/mozilla-central/rev/409f93411380

Yeah, I forgot to link the changeset. My bad.

Comment 81

7 years ago
Ok, thanks. Setting the milestone to b7 instead of b8 then.
Target Milestone: Firefox 4.0b8 → Firefox 4.0b7

Comment 82

7 years ago
(In reply to comment #76) 
> I think the blue arrow/circle should fill-in instead of spin to give a more
> accurate sense of "Loading...", I believe that's what I miss from progress
> lines now that they're gone.

Look at the title, it says 'return to using non deterministic throbbers'. That is the opposite from a (deterministic) progress bar or pie chart, where you indicate a specific amount (the total number of bytes or objects to be downloaded, etc ...). There are many ideas about it, but this bug clearly says in comment 0 why that has not been chosen :
>  - (implementation) no way to truthfully relay overall progress

This bug is fixed. You can always file a new bug if you want, and continue the discussion there. See comment 73 too.

Comment 83

7 years ago
with that this solution is better than filling bar , in that way i am able to understand when site is connecting , when loading and i dont feel lost when i am waiting initial page rendering

Comment 84

7 years ago
Great move in changing the tab throbbers, I just thought I'd share some feedback. 

Is it just me or does anyone feel that having the "connecting" animation going backwards adds a level of confusion or mistrust in the browser?

What I mean is: 
* Shunting between the two direction feels inconsistent and odd
* Having it move backwards slowly kind of gives me a feeling that something has gone wrong (connection issue, etc)
* Ruins the smooth flowing feeling of loading a page, instead of it being a smooth transitional process it adds a "jerk" to it which may detract from the users experience (or impression of the browser)
* generally, wouldn't you say moving backwards invokes feelings of negativity? 

I realise that what you're doing is trying to convey two different loading states in an obvious way, but the colour (grey / blue) and animation (slow / fast) is already pretty indicative that two different processes are going on, my honest opinion is that the third difference (forward / backward) isn't needed and is more of a detraction. 

Anyway, I am making sense and regardless I am a big fan of the artwork and the concept in general.

Comment 85

7 years ago
(In reply to comment #84)
i agree with you , the backwward animation is givin a bad indication . 
i'd suggest that keeping the blue icon and slowly replacing the blue color with someother color as the page loads .. "like green" and when the page is loaded the icon w'd be fully green , befor it hides and replaced by the favicon . 
i'd make some animation to show the idea later today .

Comment 86

7 years ago
Iy you plan to display seevral colors regarding the loading state, we should try to be consistant with the identity site button color which also has several states (orange, blue, green) if possible

Comment 87

7 years ago
The throbber looks really nice!

But I think I've found a problem: if you have got only one tab and have unchecked "always show the tab bar", you don't see the throbber at all because in this case no tab title bar exists.

Comment 88

7 years ago
I don't think it's possible to spin "backwards".

(However, the merits of this change, or futher changes, should be discussed  somewhere outside this bug.)

Comment 89

7 years ago
(In reply to comment #87)
> But I think I've found a problem: if you have got only one tab and have
> unchecked "always show the tab bar", you don't see the throbber at all because
> in this case no tab title bar exists.

This is similar to Bug 603594

Comment 90

7 years ago
Why is is it so ridiculously expensive to render a small animated icon like this. I mean I get max cpu usage (50% on my core2 cpu) with about for of them. Even worse than the ff3 throbber, which is pretty bad itself.
Oh good god, no. I can't remove myself from the cc list because I reported this bug, and people refuse to listen to comment 73. My poor, poor bugmail.
Whiteboard: THIS BUG IS CLOSED, NO MORE COMMENTS, PLEASE!
I think it is hight time to improve bugzilla A LOT. And I'm not talking just about this, but mainly from usability perspective.

Updated

7 years ago
Depends on: 604027
(In reply to comment #1)
> Note: this will not restore the verbose text information that used to display
> in the status bar; that will be dealt with separately, but I'll reference it
> once the bug is filed.

Mike, did you open that other bug ? There was some quite convincing discussion about the need of that on mozilla.dev.apps.firefox. 
And then I could attach my idea for that, which is a variation on your comment #5

Comment 94

7 years ago
(In reply to comment #93)
-> bug 603777

Comment 95

7 years ago
I don't know if it's the good place to point out that :
sometimes a web site opens another window without displaying firefox controls (no toolbar/url bar etc)
With Firefox 3.6 the status bar provides the information related to page loading.
Will the user still have the loading information with Firefox 4 ?

Comment 96

7 years ago
(In reply to comment #95)
-> bug 603594

Comment 97

7 years ago
Thank you Dave Garrett
Marking as verified fixed. Throbbers are back on all platforms.
Status: RESOLVED → VERIFIED

Updated

7 years ago
Depends on: 606037

Updated

7 years ago
No longer depends on: 603594

Comment 99

6 years ago
Verified fixed on Mozilla/5.0 (Windows NT 6.1; rv:2.2a1pre) Gecko/20110405 Firefox/4.2a1pre
Blocks: 679927
Flags: in-litmus?(abillings) → in-litmus?
You need to log in before you can comment on or make changes to this bug.