Closed Bug 1367868 Opened 3 years ago Closed 2 years ago

audio element collapses to 0 size if given bogus audio URL, with "too much recursion videocontrols.xml:1185:13"

Categories

(Toolkit :: Video/Audio Controls, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: dholbert, Assigned: ralin)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Attached file testcase 1
STR:
 1. Load attached testcase.

EXPECTED RESULTS:
 Visible audio controls (nonzero size)

ACTUAL RESULTS:
 Audio element collapses to 0 size.
(In reply to Daniel Holbert [:dholbert] (reduced availability - travel & post-PTO backlog) from comment #0)
> ACTUAL RESULTS:
>  Audio element collapses to 0 size.

Actually, in current Nightly:
 - The audio controls show up at first.
 - Then they fade out.
 - Then the element collapses to 0 size (as shown by its border collapsing to a single point).


Additionally, after the bogus mp3 load fails, I see the following JS Error in my browser console:
* too much recursion    videocontrols.xml:1185:13
https://dxr.mozilla.org/mozilla-central/rev/f81bcc23d37d7bec48f08b19a9327e93c54d37b5/toolkit/content/widgets/videocontrols.xml#1185

* too much recursion    videocontrols.xml:1485:9
https://dxr.mozilla.org/mozilla-central/rev/f81bcc23d37d7bec48f08b19a9327e93c54d37b5/toolkit/content/widgets/videocontrols.xml#1485
Flags: needinfo?(ralin)
Summary: audio element collapses to 0 size if given bogus audio URL → audio element collapses to 0 size if given bogus audio URL, with "too much recursion videocontrols.xml:1185:13"
It looks like those problem including "too much recursion" are derived from the patch in bug 1339269. I'll address this issue in bug 1362146, and add all these test cases afterwards. Thanks.
Flags: needinfo?(ralin)
Depends on: 1362146
Duplicate of this bug: 1368053
Blocks: 1368639
No longer depends on: 1362146
Hi Daniel,


> Actually, in current Nightly:
>  - The audio controls show up at first.
>  - Then they fade out.
>  - Then the element collapses to 0 size (as shown by its border collapsing to a single point).

I just found that in 52, we do collapses controls as well since this case fulfill the condition[0]: "media has never been played && got error". 
> Additionally, after the bogus mp3 load fails, I see the following JS Error in my browser console:
> * too much recursion    videocontrols.xml:1185:13
So, I think we just need to break the infinite loop caused by reflow handler here. 

I'll file another bug to discuss whether we should hide controls when encounter error especially for <audio> as there's no extra information like in <video> telling users what going on and why controls dismisses immediately.


[0]: https://hg.mozilla.org/mozilla-central/file/62c5218b7325/toolkit/content/widgets/videocontrols.xml#l629
With audio, the issue is that you can't even tell an audio element was there to start with.
(In reply to Jean-Yves Avenard [:jya] from comment #5)
> With audio, the issue is that you can't even tell an audio element was there
> to start with.

Yeah, because the audio controls hide automatically as an error occurred prior to it's able to be started, and that's what we've been doing for a long time. The janky caused by "too much recursion" seems more urgent to me, so I'd prefer to split off the visibility issue to another bug for more specific discussion.
Well, I opened a bug about the visibility (#1368053) that was marked as a duplicate of this one...
Reopened it. My fault didn't think it through at the first time :P
Assignee: nobody → ralin
Status: NEW → ASSIGNED
> Additionally, after the bogus mp3 load fails, I see the following JS Error
> in my browser console:
> * too much recursion    videocontrols.xml:1185:13
> https://dxr.mozilla.org/mozilla-central/rev/
> f81bcc23d37d7bec48f08b19a9327e93c54d37b5/toolkit/content/widgets/
> videocontrols.xml#1185
> 
> * too much recursion    videocontrols.xml:1485:9
> https://dxr.mozilla.org/mozilla-central/rev/
> f81bcc23d37d7bec48f08b19a9327e93c54d37b5/toolkit/content/widgets/
> videocontrols.xml#1485

Bug 1373537 has addressed this issue. We no longer rely on calculation of clientWidth before adjustment, so that adjustment won't trigger another synchronous reflow and cause the loop.
Comment on attachment 8883182 [details]
Bug 1367868 - Don't hide media controls when error occurred for audio even if it hasn't been played yet.

https://reviewboard.mozilla.org/r/154110/#review159626
Attachment #8883182 - Flags: review?(jaws) → review+
Thanks for the review :D
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/d1ed3af55efb
Don't hide media controls when error occurred for audio even if it hasn't been played yet. r=jaws
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d1ed3af55efb
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
See Also: → 1362146
Duplicate of this bug: 1368053
You need to log in before you can comment on or make changes to this bug.