The pause and mute controls don't work in the BeatDetektor demo 3 unless you click the top edge of the buttons

REOPENED
Assigned to

Status

()

Core
Audio/Video: Playback
--
major
REOPENED
7 years ago
2 years ago

People

(Reporter: JK, Assigned: cpearce)

Tracking

Trunk
x86
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6) Gecko/20100101 Firefox/4.0b6
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101016 Firefox/4.0b8pre

The pause and mute controls don't work in the BeatDetektor demos. Clicking them does nothing.

Reproducible: Always

Steps to Reproduce:
1. Open the URL
2. Click the play button
3. Click the pause and mute buttons
(Reporter)

Updated

7 years ago
blocking2.0: --- → ?
Keywords: useless-UI
Version: unspecified → Trunk
(Reporter)

Updated

7 years ago
Summary: The pause and mute controls don't work in the BeatDetektor demos → The pause and mute controls don't work in the BeatDetektor demo 3
Keywords: useless-UI
(Reporter)

Comment 1

7 years ago
Actually the buttons work, but only if you hit the top edge of the buttons.
(Reporter)

Updated

7 years ago
Summary: The pause and mute controls don't work in the BeatDetektor demo 3 → The pause and mute controls don't work in the BeatDetektor demo 3 unless you click the top edge of the buttons

Comment 2

7 years ago
confirmed. Are the controls supposed to be in the upper left, or is that another bug?
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 3

7 years ago
Yup the audio element is supposed to stay absolutely positioned in the top left corner and has a specific css size set; this has been a problem for awhile and seems to be related to some of the following:

1. Unnecessary audio loading animation above the element -- can't seem to turn this off or prevent it from breaking the size of the element from what I set via CSS.  Would be nice if this could be removed or styled away as I do not want or need it and it just causes layout breaks.

2. Volume slider always goes up, if the volume slider was a bit smarter it would recognize it's going to go off the top of the screen and open in another direction instead.

3. Time position marker extends past the top as well, pushing outside of what I defined via CSS sizes.  This marker should not be visible in my case; if it doesn't have room it should either extend down, hide or remain within the given size (or just put the audio time beside the bar instead like most media apps do).

Thanks!
blocking2.0: ? → final+
(Assignee)

Comment 4

7 years ago
Created attachment 485945 [details] [diff] [review]
Patch: don't paint overflow of video controls

Beatdetektor sets the height of the audio element using CSS to be less than the height required to draw the controls, and this is messing up the controls rendering.

I spoke with Charles on IRC, and he setup a copy of beatdetektor which doesn't set a CSS height, and the controls work in that example, see:
http://cubicvr.org/CubicVR.js/bd3/BeatDetektor3HD_nocssheight.html

Charles was setting the CSS height to try to stop the throbber from showing, which causes the audio element to resize. Ideally we'll replace the throbber on audio elements' controls with some other UI mechanism which doesn't require resizing the audio element when "throbbing", but that's a post FF4 thing...

As discussed with Roc, this patch changes the audio and video elements' style so that their controls will be clipped to their CSS dimensions. When the throbber is showing, the bottom of the controlsframe will be clipped when the CSS height is insufficient.
Assignee: nobody → chris
Status: NEW → ASSIGNED
Attachment #485945 - Flags: review?(roc)
Comment on attachment 485945 [details] [diff] [review]
Patch: don't paint overflow of video controls

Needs a reftest!
Attachment #485945 - Flags: review?(roc) → review+
(Assignee)

Comment 6

7 years ago
Created attachment 485947 [details] [diff] [review]
Patch: ensure controls resize correctly when status overlay hides and shows

Sometimes when the status overlay hides, we don't resize after it's faded out. There's already a work around in the controls to force a style resolution when we fade in, so we can move it to run when we fade out as well to ensure a media element is always the correct size after fade in or fade out.
Attachment #485947 - Flags: review?(dolske)
(Assignee)

Comment 7

7 years ago
Created attachment 486144 [details] [diff] [review]
Patch: add reftest

Adds reftest which ensures video controls don't overflow outside of the css-specified dimensions. I pushed this to tryserver overnight, and it passed on all platforms.
(Assignee)

Updated

7 years ago
Blocks: 538084
Comment on attachment 485947 [details] [diff] [review]
Patch: ensure controls resize correctly when status overlay hides and shows

>                     if (fadeIn) {
>                         element.setAttribute("hidden", false);
>-                        // force style resolution, so that transition begins
>-                        // when we remove the attribute.
>-                        getComputedStyle(element, "").display;
>                         element.removeAttribute("fadeout");

This unfortunately breaks the fade-in transition, instead of a fade they just blink in on mouseover... The style flush has to happen after we unhide it but _before_ the removeAttribute().

The obvious fix is to leave this bit as-is, and just add a flush in the |else| block.

While you're here, just access |element.clientTop| instead of calling getComputedStyle()? That seems to be what we're using elsewhere for this kind of thing now. I just tested that on Win7, works fine (controls fade-in as before).
Attachment #485947 - Flags: review?(dolske) → review-
(Assignee)

Comment 9

7 years ago
Created attachment 490952 [details] [diff] [review]
Patch 2: with review comments addressed

Changed as requested in review comments.
Attachment #485947 - Attachment is obsolete: true
Attachment #490952 - Flags: review?(dolske)
Attachment #490952 - Flags: review?(dolske) → review+
(Assignee)

Comment 10

7 years ago
http://hg.mozilla.org/mozilla-central/rev/00cabcc7d182
http://hg.mozilla.org/mozilla-central/rev/8d54b45a181e
http://hg.mozilla.org/mozilla-central/rev/53f49c89634b
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Reporter)

Comment 11

7 years ago
The controls are almost invisible now.
(Assignee)

Comment 12

7 years ago
(In reply to comment #11)
> The controls are almost invisible now.

Note if the video controls have a CSS specified height which is smaller than the amount of space needed to show them, now they'll be clipped to the CSS specified height. Are they visible for you in
http://cubicvr.org/CubicVR.js/bd3/BeatDetektor3HD_nocssheight.html ?
(Reporter)

Comment 13

7 years ago
(In reply to comment #12)
> (In reply to comment #11)
> > The controls are almost invisible now.
> 
> Note if the video controls have a CSS specified height which is smaller than
> the amount of space needed to show them, now they'll be clipped to the CSS
> specified height. Are they visible for you in
> http://cubicvr.org/CubicVR.js/bd3/BeatDetektor3HD_nocssheight.html ?

Yes, they are visible in that link.
(Assignee)

Comment 14

7 years ago
Backed out, due to the changes here causing bug 618203. I discussed this with roc, and we agreed not to block2.0 on this.

http://hg.mozilla.org/mozilla-central/rev/4ec43842c392
http://hg.mozilla.org/mozilla-central/rev/ea5076f3b4fe
http://hg.mozilla.org/mozilla-central/rev/36abf00aa201
http://hg.mozilla.org/mozilla-central/rev/a92cfed15419
http://hg.mozilla.org/mozilla-central/rev/fd28a949d4ce
http://hg.mozilla.org/mozilla-central/rev/a6a3200c19e5
http://hg.mozilla.org/mozilla-central/rev/608fc8fa26dc
Status: RESOLVED → REOPENED
blocking2.0: final+ → ---
Resolution: FIXED → ---
Component: Audio/Video → Audio/Video: Playback
You need to log in before you can comment on or make changes to this bug.