Closed Bug 698940 Opened 13 years ago Closed 13 years ago

Remove throbber from HTML5 audio (with controls attribute present)

Categories

(Toolkit :: Video/Audio Controls, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: jaws, Assigned: jaws)

Details

(Whiteboard: [fixed-in-fx-team])

Attachments

(1 file, 1 obsolete file)

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.
Attached patch Patch for bug 698940 (obsolete) — Splinter Review
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 />
Attachment #571204 - Flags: review?(dolske)
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 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. :)
Attachment #571204 - Flags: review?(dolske) → review-
I've fixed the issue with the previous patch and carried forward the r+ from dolske.
Attachment #571204 - Attachment is obsolete: true
Attachment #575948 - Flags: review+
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: