1290510 - Put media control notification behind a Nightly flag

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

Next week the media control notification rides the trains to the beta channel. However we are still testing and fixing bugs. Let's put the notification behind a Nightly flag and continue to improve the quality. Once we feel it's ready we can remove the Nightly flag and let it ride the trains.

Comment 1

[Alastor Wu [:alwu]]

Hi, Sebastian,
What's "put the notification behind a Nightly flag" mean?
Do we need to turn the preference off?
Thanks!

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

This means we should only execute the new code (or show the notification) if AppConstants.NIGHTLY_BUILD is true. With that only Nightly users will get the new feature. As soon as we think the feature is good enough and there are no known bugs anymore we can remove this flag again.

Comment 3

[Alastor Wu [:alwu]]

Attachment: Bug 1290510 - add nightly flag for media control.

Review commit: https://reviewboard.mozilla.org/r/68288/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/68288/

Comment 4

[William Hsu [:whsu]]

Hi, Rachelle,

It seems to me that we're going to keep this feature on Nightly and turn it off on the other branches.
Do we need to notify release engineering regarding this change?
Any thought?

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8776527 [details]
Bug 1290510 - add nightly flag for media control.

https://reviewboard.mozilla.org/r/68288/#review65330

::: mobile/android/base/java/org/mozilla/gecko/media/MediaControlService.java:261
(Diff revision 1)
>  
>          if (!mIsMediaControlPrefOn) {
>              return;
>          }
>  
> +        // TODO : remove this checking when the media control is ready to ship.

We can already file a bug for that and mention the bug number here. After that we can make all bugs we intend to fix block this one.

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

Let's not forget to request uplift for this (It's merge day today so we might need to uplift it to Beta and Aurora).

Comment 7

[Alastor Wu [:alwu]]

Comment on attachment 8776527 [details]
Bug 1290510 - add nightly flag for media control.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68288/diff/1-2/

Comment 8

[Alastor Wu [:alwu]]

Postpone to land this, let wait for the response from release engineer side first.

Comment 9

[Alastor Wu [:alwu]]

OK, let's go to land this and re-enable after all bugs are fixed :)

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/909dbdc5f047
add nightly flag for media control. r=sebastian

Comment 11

[Rachelle Yang [:ryang][[e-mail]]]

Based on our discussion, lets land Bug 1290510 and re-enable after all bugs are fixed.
Thank you, Alastor !

Comment 12

[Alastor Wu [:alwu]]

Comment on attachment 8776527 [details]
Bug 1290510 - add nightly flag for media control.

Approval Request Comment
[Feature/regressing bug #]: Only enable media control on nightly version until it's more stable and ready to ship (we would re-enable this feature on bug1290836)
[User impact if declined]: User might encounter some bugs
[Describe test coverage new/current, TreeHerder]: No UI test now
[Risks and why]: low, disable this feature on beta and aurora wouldn't cause any new bugs
[String/UUID change made/needed]: no

Comment 13

[Gerry Chang [:gchang]]

Comment on attachment 8776527 [details]
Bug 1290510 - add nightly flag for media control.

Agree with what Sebastian mentioned. Let's let it on nightly only.

Comment 14

[Gerry Chang [:gchang]]

We need QE's help to make sure this feature is only on nightly.

Comment 15

[Gerry Chang [:gchang]]

Hi Barbara,
If we only let this feature ride the train on 51 and not on 49 & 50, will you have other concerns?

Comment 16

[Barbara Bermes [:barbara] - NI please!]

No, all good Gerry, I rather have a well tested and bug-free feature in the product.

Comment 17

[Gerry Chang [:gchang]]

Hi Barbara,
Thanks. Then, there is no need to uplift all the related fixes to 49 & 50.

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/909dbdc5f047

Comment 19

[Barbara Bermes [:barbara] - NI please!]

I'm a bit confused now, can you share some screenshots of what kind of media control notification we are talking about and how it relates to https://bugzilla.mozilla.org/show_bug.cgi?id=1249579.

Reading the paragraph on the blog here: https://blog.mozilla.org/blog/2016/08/02/exciting-improvements-in-firefox-for-desktop-and-android/, I was under the impression the handling of audio/media controls are actually in 48.

Maybe we are talking about two different things here.

Comment 20

[Sebastian Kaspari (:sebastian; :pocmo)]

In 48 we shipped the new in-app video controls (bug 1250741):
https://bug1250741.bmoattachments.org/attachment.cgi?id=8738481

This is just about the notification with controls, which was new in 49 and which needs some bugfixes before we can let it ride the trains:
https://pbs.twimg.com/media/CkWhKGrWUAQ8KuD.jpg

Comment 21

[Barbara Bermes [:barbara] - NI please!]

Got it, thanks Sebastian, that's what I hoped/assumed.

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d519ced01b56

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/36ee19db749c

Comment 24

[Sorina Florean [:sflorean]]

Tested using device:
- LG G4 (Android 5.1)
- Nexus 5 (Android 6.0.1)

On Nightly 51.0a1 (2016-08-08) media content notification is displayed when audio content is played.
On Aurora 50.0a2 (2016-08-08) media content notification is not displayed when audio content is played.
On Firefox 49.0b1 media content notification is displayed.

Comment 25

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Sorina Florean [:sorina] from comment #24)
> On Firefox 49.0b1 media content notification is displayed.

The code landed in mozilla-beta[1], maybe too late for Beta 1, let's see if it's disabled in Beta 2.

[1] https://hg.mozilla.org/releases/mozilla-beta/rev/36ee19db749c

Comment 26

[Sorina Florean [:sflorean]]

(In reply to Sebastian Kaspari (:sebastian) from comment #25) 
> The code landed in mozilla-beta[1], maybe too late for Beta 1, let's see if
> it's disabled in Beta 2.
> 
> [1] https://hg.mozilla.org/releases/mozilla-beta/rev/36ee19db749c

Thanks, I will set the flag for 50 and 51 verified and test again when Beta 2 is landed.

Comment 27

[u421692]

Verified as fixed on Firefox 49 Beta 2

Comment 28

[Stefan Deiac]

Based on last updates I will remove the qe-verify flag, thanks.