Don't show 'play tab' icon for video without audio track

VERIFIED FIXED in Firefox 52

Status

()

Core
Audio/Video: Playback
VERIFIED FIXED
a year ago
9 months ago

People

(Reporter: alwu, Assigned: alwu)

Tracking

Other Branch
mozilla53
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 unaffected, firefox51 unaffected, firefox52 verified, firefox53 verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

Fork from bug1318462 comment4.

---

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20161122030216

Play tab button is also shown when opening "Get Firefox for Android" thumbnail from the New tab.

STR:
1. Launch Firefox with a new profile
2. Open a new tab
3. Click on the "Get Firefox for Android" thumbnail.

Expected results:
The Play tab button should not be displayed in the opened tab.

Actual results:
Tab play button is displayed, even if there's no media to play and the tab is not in the foreground
We should check whether the blocked media element has audio or not, we don't need to send the notification if it doesn't have audio track.
Comment hidden (mozreview-request)
Ah, comment1 didn't work well because we don't get the metadata at that time...
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Depends on: 1309162
Comment hidden (mozreview-request)

Comment 8

a year ago
mozreview-review
Comment on attachment 8813982 [details]
Bug 1320005 - don't show the 'play tab' icon for the media element without audio track.

https://reviewboard.mozilla.org/r/95290/#review98536

::: dom/audiochannel/AudioChannelService.h:69
(Diff revision 5)
>    NS_DECL_NSIOBSERVER
>    NS_DECL_NSIAUDIOCHANNELSERVICE
>  
> -  enum AudibleState : bool {
> -    eAudible = true,
> -    eNotAudible = false
> +  /**
> +   * eNotAudible : agent is no audible
> +   * eMaybeAudible : agent is no audible now, but it might be audible later

is not

::: dom/html/HTMLMediaElement.cpp:889
(Diff revision 5)
> -      return false;
> +      return AudioChannelService::AudibleState::eNotAudible;
>      }
>  
> -    // No sound can be heard during suspending.
> -    if (IsSuspended()) {
> -      return false;
> +    // No audio track.
> +    if (!mOwner->HasAudio()) {
> +      return AudioChannelService::AudibleState::eNotAudible;;

double ;;

::: dom/html/HTMLMediaElement.cpp:6741
(Diff revision 5)
>  HTMLMediaElement::SetMediaInfo(const MediaInfo& aInfo)
>  {
> +  const bool oldHasAudio = mMediaInfo.HasAudio();
>    mMediaInfo = aInfo;
>    if (mAudioChannelWrapper) {
>      mAudioChannelWrapper->AudioCaptureStreamChangeIfNeeded();

Write a comment explaining why HasAudio() can change value here.
Attachment #8813982 - Flags: review?(amarchesini) → review+
Comment hidden (mozreview-request)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4e431051d8bcbab15dc7ae7058b772f76c9d7aaf
Comment hidden (mozreview-request)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cb1d2f13506e21346aab851ec0284ca1e7843556&selectedJob=32591641

Comment 13

a year ago
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c815adcd0cb8
don't show the 'play tab' icon for the media element without audio track. r=baku

Comment 14

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c815adcd0cb8
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Duplicate of this bug: 1318462
Please request Aurora approval on this when you feel it has baked for sufficiently long on Nightly.
status-firefox50: --- → unaffected
status-firefox51: --- → unaffected
status-firefox52: --- → affected
Flags: needinfo?(alwu)
Created attachment 8819263 [details] [diff] [review]
Bug 1320005 - don't show the 'play tab' icon for the media element without audio track. (for aurora)

Approval Request Comment
[Feature/Bug causing the regression]: The play icon would be showed in some wrong situations
[User impact if declined]: user would see the wrong unblocking icon and it won't disappear anymore
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: no
[Is the change risky?]: no
[Why is the change risky/not risky?]: only affect icon display cases
[String changes made/needed]: no
Flags: needinfo?(alwu)
Attachment #8819263 - Flags: review+
Attachment #8819263 - Flags: approval-mozilla-aurora?
Comment on attachment 8819263 [details] [diff] [review]
Bug 1320005 - don't show the 'play tab' icon for the media element without audio track. (for aurora)

fix new a/v issue in aurora52
Attachment #8819263 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 19

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/4d428919f82f
status-firefox52: affected → fixed

Updated

a year ago
See Also: → bug 1328058
Verified as fixed using the latest Nightly 53.0a1 (Build ID: 20170108030212)and the latest Developer Edition 52.0a2 (Build ID: 20170108004005) on Windows 10 x64 bit, Mac OS X 10.11 and Ubuntu 16.04 x64 bit.

The play tab indicator is not displayed anymore when using the scenario from the Description or the scenario from https://bugzilla.mozilla.org/show_bug.cgi?id=1318462#c0.
Status: RESOLVED → VERIFIED
status-firefox52: fixed → verified
status-firefox53: fixed → verified
Blocks: 1308153
You need to log in before you can comment on or make changes to this bug.