Tab audio indicator doesn't show up on the huffingtonpost Parkersburg article

VERIFIED FIXED in Firefox 42

Status

()

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

unspecified
mozilla43
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 verified, firefox43 verified)

Details

Attachments

(1 attachment)

This happens because the page calls .play() on the video element before it has its metadata loaded, so HasAudio() returns false when we register the audio channel agent and we end up not emitting the audio-playback notification.  We need to deal with HasAudio() changing correctly in MetadataLoaded.
Component: Tabbed Browser → Audio/Video
Product: Firefox → Core
Comment on attachment 8654943 [details] [diff] [review]
Send the audio-playback notification when the page calls HTMLMediaElement::Play() before the metadata has been fully loaded

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

::: dom/html/HTMLMediaElement.cpp
@@ +4519,5 @@
> +{
> +  if (!mAudioChannelAgent) {
> +    nsresult rv;
> +    mAudioChannelAgent = do_CreateInstance("@mozilla.org/audiochannelagent;1", &rv);
> +    if (!mAudioChannelAgent) {

we should actually use 'rv' here. something like:

if (NS_WARN_IF(NS_FAILED(rv))) {
  return false;
}

MOZ_ASSERT(mAudioChannelAgent);

::: dom/html/HTMLMediaElement.h
@@ +58,5 @@
>  
>  class nsITimer;
>  class nsRange;
>  class nsIRunnable;
> +class AutoNotifyAudioChannelAgent;

alphabetic order.

@@ +1049,5 @@
>    void UpdateReadyStateInternal();
>  
>    // Notifies the audio channel agent when the element starts or stops playing.
>    void NotifyAudioChannelAgent(bool aPlaying);
> +  friend class AutoNotifyAudioChannelAgent;

move this at the beginning of the class definition.
Attachment #8654943 - Flags: review?(amarchesini) → review+
Comment on attachment 8654943 [details] [diff] [review]
Send the audio-playback notification when the page calls HTMLMediaElement::Play() before the metadata has been fully loaded

Approval Request Comment
[Feature/regressing bug #]: Tab audio indicator feature polish
[User impact if declined]: See comment 0.  The audio indicator may not appear on some websites that play audio.
[Describe test coverage new/current, TreeHerder]: Tested locally and has automated tests.
[Risks and why]: Pretty self-contained change, should not be very risky. 
[String/UUID change made/needed]: None.
Attachment #8654943 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/e90965135940
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Depends on: 1200673
Comment on attachment 8654943 [details] [diff] [review]
Send the audio-playback notification when the page calls HTMLMediaElement::Play() before the metadata has been fully loaded

Polish a new feature, taking it.
Attachment #8654943 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified fixed using latest Nightly 43.0a1 (buildID: 20150913030204) and latest Aurora 42.0a2 (buildID: 20150914004020).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.