Closed
Bug 1320005
Opened 8 years ago
Closed 7 years ago
Don't show 'play tab' icon for video without audio track
Categories
(Core :: Audio/Video: Playback, defect)
Tracking
()
VERIFIED
FIXED
mozilla53
Tracking | Status | |
---|---|---|
firefox50 | --- | unaffected |
firefox51 | --- | unaffected |
firefox52 | --- | verified |
firefox53 | --- | verified |
People
(Reporter: alwu, Assigned: alwu)
References
Details
Attachments
(2 files)
58 bytes,
text/x-review-board-request
|
baku
:
review+
|
Details |
Bug 1320005 - don't show the 'play tab' icon for the media element without audio track. (for aurora)
12.05 KB,
patch
|
alwu
:
review+
jcristau
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•8 years ago
|
||
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) |
Assignee | ||
Comment 3•8 years ago
|
||
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) |
Comment hidden (mozreview-request) |
Comment 8•7 years 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) |
Assignee | ||
Comment 10•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4e431051d8bcbab15dc7ae7058b772f76c9d7aaf
Comment hidden (mozreview-request) |
Assignee | ||
Comment 12•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cb1d2f13506e21346aab851ec0284ca1e7843556&selectedJob=32591641
Comment 13•7 years 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•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c815adcd0cb8
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Comment 16•7 years ago
|
||
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)
Assignee | ||
Comment 17•7 years ago
|
||
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 18•7 years ago
|
||
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•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/4d428919f82f
Comment 20•7 years ago
|
||
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.
You need to log in
before you can comment on or make changes to this bug.
Description
•