698940 - Remove throbber from HTML5 audio (with controls attribute present)

Status: RESOLVED FIXED

(Toolkit :: Video/Audio Controls, defect)

Description

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

Until we have a throbber within the audio elements box, we need to remove the throbber because it resizes the element and leaves an awkward amount of space around the element after the throbber disappears.

Comment 1

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

Attachment: Patch for bug 698940

This patch removes the throbber from audio elements.

Note that there is a follow-up bug necessary for standalone audio files, since VideoDocument.cpp (which audio files use) creates a <video> element even when viewing <audio>.

<video> elements seem to have extra height to them by default.

See these examples:
data:text/html, <audio src="http://upload.wikimedia.org/wikipedia/commons/1/1d/Demo_chorus.ogg" controls />
data:text/html, <video src="http://upload.wikimedia.org/wikipedia/commons/1/1d/Demo_chorus.ogg" controls />

Comment 2

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

Frank showed me that the default sizes for video elements is set here:
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsVideoFrame.cpp#577

We may be able to set the intrinsic height and width on standalone <video> elements if we determine them to be audio only.

Comment 3

[Justin Dolske [:Dolske]]

Comment on attachment 571204 [details] [diff] [review]
Patch for bug 698940

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

::: toolkit/content/widgets/videocontrols.xml
@@ +348,5 @@
>                          show = true;
>  
> +                    // Explicitly hide the status fader if this
> +                    // is audio only until bug 619421 is fixed.
> +                    show = !this.isAudioOnly;

Uhm. I think what you actually want here is:

  if (this.isAudioOnly)
    show = false;

r+ with that. :)

Comment 4

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

Attachment: Patch for bug 698940 (for checkin)

I've fixed the issue with the previous patch and carried forward the r+ from dolske.

Comment 5

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

https://hg.mozilla.org/integration/fx-team/rev/c62f0aacc6b6

Comment 6

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/mozilla-central/rev/c62f0aacc6b6