Closed Bug 534409 Opened 12 years ago Closed 8 years ago

Improper placement of html 5 audio control bar via css position

Categories

(Toolkit :: Video/Audio Controls, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: mpeters, Assigned: mjh563)

References

()

Details

(Whiteboard: [good first bug][mentor=jaws][lang=css])

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.7pre) Gecko/20091204 Ubuntu/9.04 (jaunty) Shiretoko/3.5.7pre
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.7pre) Gecko/20091204 Ubuntu/9.04 (jaunty) Shiretoko/3.5.7pre

When positioning html 5 audio control bar via CSS, FireFox incorrectly places the control bar lower than it should.

While the audio is buffering, it places it correctly, but the control bar sinks once the audio has buffered.

I suspect (but do not know) it is related to the progress bubble on the control bar that sticks up above.

Reproducible: Always

Steps to Reproduce:
1. use css to place an audio control bar at bottom of a div
2. load page
3.
Actual Results:  
control bar sinks to below bottom of div

Expected Results:  
control bar at bottom of div

firefox correctly place FlowPlayer control bar.
Google Chrome correctly places both audio and flowplayer
Have not tested to see what Safari does.

I would like this fixed, it impacts a currently live web site of mine.
This seems fixed with the latest trunk nightly build on Windows, but not with Firefox 3.5 and Firefox 3.6 nightly (Namoroko). Although the flash bar design looks better.
Trunk builds are here: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-central/
Component: General → Video/Audio
Product: Firefox → Core
QA Contact: general → video.audio
Version: unspecified → 1.9.1 Branch
The bug still exists for me in Minefield 3.7a1pre (x86_64 Linux)

Sometimes the bottom of the control bar gets cut off after loading the buffer making it appear to look like it is doing the right thing but when you play it shows up again.

http://www.shastaherps.org/bugs/minefield.png

Is screen shot showing bug in Minefield.
Yeah, you're right. If I scroll the control in 3.5 and 3.6 builds out of sight the bar sinks and is no longer cut off. And then they look like trunk.
With trunk the bar is lower but painting problems are solved. On Windows at least.
Component: Video/Audio → Video/Audio Controls
Product: Core → Toolkit
QA Contact: video.audio → video.audio
Version: 1.9.1 Branch → unspecified
Assignee: nobody → fryn
Unassigning myself, since I likely won't be working on this in the near future. Patches are welcome! :)

I do plan to investigate this at some point though, as hacking CSS is often a fun challenge :P
Assignee: fryn → nobody
I can confirm this bug on OSX / FF4.

Screenshots for Safari, FF4, Chrome.

The page contains an audio tag and a pre tag, and a style tag with the rules as shown.
Oops forgot to link the screenshots:

http://imgur.com/a/IpJH4
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: x86_64 → All
Version: unspecified → Trunk
As Frank mentioned earlier, this is probably a bug in our CSS. The CSS file for this is at /toolkit/themes/{win,pin}stripe/global/media/videocontrols.css

There is also another CSS file at /toolkit/content/widgets/videocontrols.css
Whiteboard: [good first bug][mentor=jaws][lang=css]
The bug seems to still exist in the latest trunk, I'll try to fix it. Can you please assign me on it?
Assignee: nobody → dan.p.ja
Are you actively working on this, Dan?
Flags: needinfo?(dan.p.ja)
Yes. I'm sorry, I was very busy recently. I will work on it after this weekend.
Flags: needinfo?(dan.p.ja)
Dan, can you confirm that you're still working on this? If not, it would be good to free this up to allow someone else to solve it.
Flags: needinfo?(dan.p.ja)
Yes I am. I got some advices from jaws, I hope I will be able to solve it soon. If not, I'll free it for someone else.
Flags: needinfo?(dan.p.ja)
OK, here is what I found:

It seems to me, that the bug is caused by improper height of audio element. There is a file  toolkit/themes/windows/global/media/videocontrols.css, where the contents of audio tags are styled. Basically everything is inside .controlBar, that has height 28px, same as other elements inside audio. However audio tag itself seems to have the height only 25px, but I didn't manage to find any source, that could cause the problem.
There is also toolkit/content/widgets/videocontrols.css but there are mostly CSS transitions, nothing that could cause the bug. There are not any other css files that contain any definition for audio.

I have tried to set the height of audio in layout/style/html.css: 

audio {min-height:28px}

and it seems to fix the bug, but I guess that's not a good solution since it imho doesn't solve the source of the problem. Is it possible to fix it only as suggested? If not, do you have any idea where else the problem could be?
Flags: needinfo?(jaws)
Status: NEW → ASSIGNED
I placed a breakpoint in nsVideoFrame::ComputeSize and the size returned shows 28 CSS pixels (1680 application units). However both DOM Inspector and the built-in devtools show a computed height for the <audio> tag as 21 CSS pixels. So somewhere 7 pixels get lost. The scrubber thumb sits outside of the controlbar by the same 7 pixels, so the problem most likely lies with this.

I'll continue to look deeper here.
Flags: needinfo?(jaws)
I think the problem is happening because the height of the control gets sized in proportion to its width. The default size seems to be 300 x 28, but if you set a width smaller than 300px then the height is reduced and this is what causes it to be pushed down.

The opposite happens with the example shown in comment 7. Here, the width of the control is greater than 300px, which increases the height and so you get a gap.

I'll attach a testcase that shows how it appears at various widths.
Attached file testcase
In this testcase the <audio> elements have a red border.

The 300px wide element is the only one with the correct/expected height.
Attachment #765780 - Attachment mime type: text/plain → text/html
Dan and Jaws, are either of you currently working on this?  If not, please re-assign the bug to me and I'll try to fix it.
Flags: needinfo?(dan.p.ja)
Assignee: dan.p.ja → nobody
Flags: needinfo?(dan.p.ja)
So... the height of the element is set in nsLayoutUtils::ComputeSizeWithIntrinsicDimensions, which is called from nsVideoFrame::ComputeSize. When the element has a specified width but an auto height, this function scales the height so that the intrinsic ratio of the element is maintained.

That may be appropriate for a video, but for an audio element containing only a fixed-height control bar it makes no sense to maintain the aspect ratio. The height should instead just be set to the intrinsic height in that case.

So I think that if it's an audio element, something different needs to be done in nsVideoFrame::ComputeSize.

(The problem also happens the other way round, when the element has a specified height and an auto width. Then, the width gets scaled instead of being set to the intrinsic width, which I think would be preferable.)
(In reply to mjh563 from comment #19)
> The height should instead just be set to the intrinsic height in that
> case.

...which can be done by passing an intrinsic ratio of nsSize(0, 0) to ComputeSizeWithIntrinsicDimensions. So I think the fix for this is quite straightforward.
Attached patch patchSplinter Review
This patch makes the following changes to layout/generic/nsVideoFrame.cpp:

(1) In ComputeSize, only set the intrinsic ratio for video elements. Audio elements are fixed height so they shouldn't have an intrinsic ratio.

(2) In GetIntrinsicRatio and GetVideoIntrinsicSize, simplify the logic so that GetVideoIntrinsicSize is no longer used to return an intrinsic ratio in the case of audio elements. (This change isn't required to fix the bug, but I think it makes the code clearer.)

Not sure who to request review from though...
Thanks for the patch!

When blaming the file, I think that Chris Double should review this, he wrote all the code around the line you touched. If not, maybe roc could review as well, since this deals with layout.
Attachment #766638 - Flags: review?(chris.double)
Attachment #766638 - Flags: review?(chris.double) → review+
Assignee: nobody → mjh563
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/58774d8692cf
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Duplicate of this bug: 897406
You need to log in before you can comment on or make changes to this bug.