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

VERIFIED FIXED in Firefox 42

Status

()

VERIFIED FIXED
3 years ago
3 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)

(Assignee)

Comment 1

3 years ago
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
(Assignee)

Comment 2

3 years ago
Created attachment 8654943 [details] [diff] [review]
Send the audio-playback notification when the page calls HTMLMediaElement::Play() before the metadata has been fully loaded
Attachment #8654943 - Flags: review?(amarchesini)
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+
(Assignee)

Comment 4

3 years ago
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
Last Resolved: 3 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Depends on: 1200673
status-firefox42: --- → affected
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
status-firefox42: fixed → verified
status-firefox43: fixed → verified
You need to log in before you can comment on or make changes to this bug.