481082 - Video controls should listen for |stalled| event

Status: RESOLVED FIXED

(Toolkit :: Video/Audio Controls, defect)

Description

[Justin Dolske [:Dolske]]

Spun off from bug 470983.

When a video fires a |stalled| event (because it's waiting on data from the network, but nothing in coming in), it would be good to show some UI to indicate this state. We should already be showing the throbber by the time |stalled| fires, but it seems like we should differentiate between "data's coming in, sit tight and we'll be right with you" and "aww, crap, nothing's happening, go grab some coffee and a Snicker's bar".

Possible ideas:
 * Add text below the throbber. EG "Waiting", "Server not responding", etc.
 * Replace fast-spinning throbber with a slow-spinning throbber?

Localization of the text will be a bit of a pain, because the unprivileged video control code can't just load up a string bundle (as normal chrome would do). XUL does generated content now (on 1.9.2, at least, via bug 472500), so it might suffice to pull in some localized CSS.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug 481082

With this patch, the throbber will spin backwards with the occurrence of the |stalled| event. We may want to change the graphic (or slow down the spinning).

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug 481082

With this patch, the throbber will spin backwards with the occurrence of the |stalled| event. We may want to change the graphic (or slow down the spinning).

[Previous patch was missing updates for pinstripe theme]

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug 481082

Updated the graphic to be a slower spinning throbber moving in the backwards direction.

Comment 4

[Justin Dolske [:Dolske]]

Comment on attachment 545792 [details] [diff] [review]
Patch for bug 481082

Review of attachment 545792 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/content/widgets/videocontrols.xml
@@ +315,5 @@
>                  setupStatusFader : function(immediate) {
>                      var show = false;
>                      if (this.video.seeking ||
>                          this.video.error ||
> +                        this.stalled ||

I don't think we want to check for stalled here, since you could be both in a stalled state and currently playing from a previously-buffered region.

We need to track the stalled state, but can just rely on the existing "waiting" event handling to trigger the throbber overlay.

Since there are a lot of other events setting the overlay type to "throbber", I suspect it would be easiest to just set an attribute ("stalled") on the throbber overlay (upon receiving stalled, and clear it upon receiving progress). And let CSS handle which image to show.

EG:

.statusIcon[type="throbber"] {
    background: url(chrome://global/skin/media/throbber.png) no-repeat center;
}
.statusIcon[type="throbber"][stalled] {
    background-image: url(stalled.png);
}

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug 481082 v2

I made the recommended changes and was able to remove the |this.stalled| property since it was redundant.

Comment 6

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/ec66137aed05

Comment 7

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/ec66137aed05