Patryk Adamczyk [:patryk] UX Reporter
	Description • 10 years ago

For the visual design refresh we have created a new set of ringtones to better reflect the visual style. These cover:
+ ringtones
+ alarms
+ message notifications

1. The WAV files are here: https://mozilla.box.com/s/6x15jupkdv98qbrrsgpy they need to be compressed. 
2. Current tones need to be replaced within the OS.

	Patryk Adamczyk [:patryk] UX Reporter
	Comment 1 • 10 years ago

Pavel can you add the tones in this link to the builds: https://mozilla.box.com/s/mnekb49udsazzjpxj3f5
1. Remove all ringtones and replacement with what is the "Ringtones" folder.
2. Make "ringer_firefox" the default
3. Remove all alarms and replacement with what is the "Alarms" folder.
4. Make "ac_awake" the default
5. Add all the new notification tones, keep the older ones

	Pavel Ivanov [:ivanovpavel][:pivanov] UX Assignee
	Comment 2 • 10 years ago

Hey Patryk,
can you check this one to be sure that everything is OK

	Patryk Adamczyk [:patryk] UX Reporter
	Comment 3 • 10 years ago

I get this error: make: *** [operatorvariant] Error 3
when I try to "make reset-gaia"

	Pavel Ivanov [:ivanovpavel][:pivanov] UX Assignee
	Comment 4 • 10 years ago

Did you try to unlock the device and then flash it?

	Patryk Adamczyk [:patryk] UX Reporter
	Comment 5 • 10 years ago

Pavel, can you have a look at this? Maybe we need to split this in 3 patches. 
Ringtones is #1 priority.
Lets make 2 separate bugs for notifications and alarms.

	Pavel Ivanov [:ivanovpavel][:pivanov] UX Assignee
	Comment 6 • 10 years ago

Sure :)
PR for Ringtones is updated now. Also we have Bug 1023133 for Notifications and Bug 1023134 for Alarms

	Patryk Adamczyk [:patryk] UX Reporter
	Comment 7 • 10 years ago

This still gives me:

make: *** [operatorvariant] Error 3
when I try to "make reset-gaia"

This happens right at the end and messes up my phone so that I have to reflash it with a fresh build.

	Stephany Wilkes
	Comment 8 • 10 years ago

Comment on attachment 8435770 [details]
patch for Gaia/master

Flagging Tif on ui-review? for the IxD side and anything else she feels like mentioning. :)

	Jim Porter (:squib)
	Comment 9 • 10 years ago

This needs new strings before anything here is going to work. Look in shared/locales/sounds/

	Tiffanie Shakespeare [:tif]
	Comment 10 • 10 years ago

Comment on attachment 8435770 [details]
patch for Gaia/master

As Jim mentioned, the patch is resulting in empty lists. LMK when it's working and  I'll ui-review. Thanks guys!

	Pavel Ivanov [:ivanovpavel][:pivanov] UX Assignee
	Comment 11 • 10 years ago

Hey Tiffanie,
based on Jim's comment I made some changes and I think it works now but please check it again because I'm not familiar with this part of Gaia

Thanks :)

	Patryk Adamczyk [:patryk] UX Reporter
	Comment 12 • 10 years ago

Still get the same error. :(

	Tiffanie Shakespeare [:tif]
	Comment 13 • 10 years ago

Comment on attachment 8435770 [details]
patch for Gaia/master

Ringtones portion of it looks good. Text is now showing up in both the manage list and the ringer selection list.

One question I have is on the previous screen, Sound, in the Ringer button we show the name of the current selection. This works in master. But in this patch, I notice that it's reverted to the "Change". Will that get resolved when this patch is merged?

	Jim Porter (:squib)
	Comment 14 • 10 years ago

The patch still doesn't change everything that needs to be changed, unfortunately, which is causing the issue in comment 13. If you grep the source tree for "ringer_", you should find all the necessary places. For completeness, here they are from my tree:

build/test/unit/settings.test.js
111:        'ringer_classic_courier.opus';
116:                       {l10nID: 'ringer_classic_courier'});
118:                   'builtin:ringer_classic_courier');
120:                   'builtin:ringer_classic_courier');
275:          'dialer.ringtone.name': {l10nID: 'ringer_classic_courier'},
276:          'dialer.ringtone.id': 'builtin:ringer_classic_courier',
277:          'dialer.ringtone.default.id': 'builtin:ringer_classic_courier',
308:          'dialer.ringtone.name': {l10nID: 'ringer_classic_courier'},
309:          'dialer.ringtone.id': 'builtin:ringer_classic_courier',
310:          'dialer.ringtone.default.id': 'builtin:ringer_classic_courier',
339:          'dialer.ringtone.name': {l10nID: 'ringer_classic_courier'},
340:          'dialer.ringtone.id': 'builtin:ringer_classic_courier',
341:          'dialer.ringtone.default.id': 'builtin:ringer_classic_courier',
373:          'dialer.ringtone.name': {l10nID: 'ringer_classic_courier'},
374:          'dialer.ringtone.id': 'builtin:ringer_classic_courier',
375:          'dialer.ringtone.default.id': 'builtin:ringer_classic_courier',

build/settings.js
65:  // Grab ringer_classic_courier.opus and convert it into a base64 string
67:  let ringtone_name = 'ringer_classic_courier.opus';

apps/ringtones/test/unit/system_tones_test.js
106:        defaultIDs.ringtone = 'builtin:ringer_classic_courier';

apps/settings/js/sound.js
140:          'ringer_classic_courier.opus';

shared/locales/sounds/sounds.en-US.properties
3:ringer_classic_prism = Classic Prism
4:ringer_classic_courier = Classic Courier
5:ringer_classic_touchmatic = Classic Touchmatic
6:ringer_classic_electric = Classic Electric
7:ringer_classic_wallphone = Classic Wallphone
8:ringer_disco_drive = Disco Drive
9:ringer_vamos_la_elektro = Vamos Là Elektro
10:ringer_digitalascent = Analog Ascent
11:ringer_low_bit_swing = Low Bit Swing
12:ringer_ringing_gems = Ringing Gems
13:ringer_rain_echoes = Rain Echoes
14:ringer_progressive_dapple = Progressive Dapple
15:ringer_digital_dapple = Analog Dapple
16:ringer_loud_windchimes = Wind Chimes
17:ringer_bitbounce = Bit Bounce

Sorry for not being clearer about this earlier.

	Pavel Ivanov [:ivanovpavel][:pivanov] UX Assignee
	Comment 15 • 10 years ago

Hey Jim,
thanks for the help I update the patch based on you comments ... just have one question ... I need to look only on build/ dir right? because I find that we also have build_stage/? Because I need to change the default Alarm tone in other bug and I think I need to follow the same logic as here

Big Thanks :)

	Jim Porter (:squib)
	Comment 16 • 10 years ago

I'm pretty sure build_stage is just where we throw files that are generated *during* the build, so you don't need to touch them (I could be wrong though).

	Patryk Adamczyk [:patryk] UX Reporter
	Comment 17 • 10 years ago

Comment on attachment 8435770 [details]
patch for Gaia/master

Pavel, finally got this patch working thanks to Tif. Looks like a good start, the ringtones seem to be in okay, however we still need to add the alarms and assign them correctly.

	Jim Porter (:squib)
	Comment 18 • 10 years ago

Changing the alias again because the current one makes it impossible for me to quicksearch for ringtones bugs. :)

	Patryk Adamczyk [:patryk] UX Reporter
	Comment 19 • 10 years ago

Comment on attachment 8435770 [details]
patch for Gaia/master

Looks good, push them in!

	David Flanagan [:djf]
	Comment 20 • 10 years ago

Comment on attachment 8435770 [details]
patch for Gaia/master

Passing review to Jim

	David Flanagan [:djf]
	Comment 21 • 10 years ago

Patryk,

Why is this bug marked confidential?

	Jim Porter (:squib)
	Comment 22 • 10 years ago

Comment on attachment 8435770 [details]
patch for Gaia/master

Like in the other bug, we're going to have to decide what to do here about migration of the default ringer. Should we just reset everyone to use one of the new ringtones?

	Patryk Adamczyk [:patryk] UX Reporter
	Comment 23 • 10 years ago

Unmarked confidential, looks like I cloned it from a confidential bug.

	Pavel Ivanov [:ivanovpavel][:pivanov] UX Assignee
	Comment 24 • 10 years ago

Hey Patryk,
what do you think about Jim's comment?

	Patryk Adamczyk [:patryk] UX Reporter
	Comment 25 • 10 years ago

Since this is a V2 product, I think its perfectly fine to reset people's tones. We will provide a link to download the v.1 tones, but the v.2 tones are so much better, I believe most people won't mind.

	Pavel Ivanov [:ivanovpavel][:pivanov] UX Assignee
	Comment 26 • 10 years ago

Hey Jim,
based on Patryk's comment ... do we need to do something?

	Jim Porter (:squib)
	Comment 27 • 10 years ago

Yes. We'll need to add a migration step that sets the ringtone to the new default upon upgrade to 2.0. I'm not sure how to do this, though.

	Pavel Ivanov [:ivanovpavel][:pivanov] UX Assignee
	Comment 28 • 10 years ago

huh ... me neither ... do you know who can help with this?

	Jim Porter (:squib)
	Comment 29 • 10 years ago

Yuren: you know about the build system, right? Do you know what the best way would be to change all users' ringtones to a new default when they upgrade to v2.0?

	Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)
	Comment 30 • 10 years ago

if you mean OTA, it's not a part of build system. for build system you can just replace the old one. schien may know about OTA.

	Jim Porter (:squib)
	Comment 31 • 10 years ago

Yeah, OTA is probably what we want. Thanks!

	Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)
	Comment 32 • 10 years ago

For settings stored in indexedDB, application developers need to explicitly write code for data migration by detecting the indexedDB version number. However, rington setting is stored via Settings API which has no version number. @arthur, can you help answer the question in comment #29?

	Arthur Chen [:arthurcc][inactive after 6/30]
	Comment 33 • 10 years ago

Currently there is no version number for settings DB as suggested in comment 32. Regarding the case of ringtone setting, what I can think of is to add code that maintains the version number of ringtone setting in system app. By doing this we could have a chance to update default ringtones at the first restart after OTA. In other cases of settings migration (ex: voicemail) we use similar ways.

	Pavel Ivanov [:ivanovpavel][:pivanov] UX Assignee
	Comment 34 • 10 years ago

Hey Arthur,
do you know who can help us with this?

	Stephany Wilkes
	Comment 35 • 10 years ago

This is not a blocker. When a patch is ready, please ensure it gets approval?/+ so that it can be uplifted for 2.0 when ready. Thanks!

	Arthur Chen [:arthurcc][inactive after 6/30]
	Comment 36 • 10 years ago

Add Dominic in as he may know things about OTA. 

Dominic, it seems we could just reset the default ringtones per comment 25.

	Dominic Kuo [:dkuo]
	Comment 37 • 10 years ago

(In reply to Arthur Chen [:arthurcc] from comment #36)
> Add Dominic in as he may know things about OTA. 
> 
> Dominic, it seems we could just reset the default ringtones per comment 25.

Although Settings API does not provide version number, but we could use some other schemes to manage the ringtone's versions from 2.0 in the ringtone app, maybe use asyncStorage to keep the version number if we will have further ringtones update in 3.0, 4.0...

The migration part could be an separated issue and we can fix it as a followup, because I feel there are more details we have to consider, such as Patryk's suggestion in comment 25 and it should related to the implementation, then this one should be clean for visual update and at least we are sure the 2.0 branch works on the new devices that are not OTA from old versions. make sense?

	Tiffanie Shakespeare [:tif]
	Comment 38 • 10 years ago

Comment on attachment 8438313 [details]
Shot from Device

clearing ui-review flag - I ui-reivew +'d the patch.

	Jim Porter (:squib)
	Comment 39 • 10 years ago

(In reply to Dominic Kuo [:dkuo] from comment #37)
> (In reply to Arthur Chen [:arthurcc] from comment #36)
> > Add Dominic in as he may know things about OTA. 
> > 
> > Dominic, it seems we could just reset the default ringtones per comment 25.
> 
> Although Settings API does not provide version number, but we could use some
> other schemes to manage the ringtone's versions from 2.0 in the ringtone
> app, maybe use asyncStorage to keep the version number if we will have
> further ringtones update in 3.0, 4.0...

The main thing I want is the ability to run some sort of migration step immediately when OTA happens. The build system should handle things for new users, but for existing users, we need to switch to the new ringtones immediately so that any incoming calls will use the new sounds. I'm just not sure how to do that, if it's even possible.

I don't think storing version info in the ringtones app will help, since we should be running this upgrade process even if the user never opens the ringtones app.

	Dominic Kuo [:dkuo]
	Comment 40 • 10 years ago

(In reply to Jim Porter (:squib) from comment #39)
> The main thing I want is the ability to run some sort of migration step
> immediately when OTA happens. The build system should handle things for new
> users, but for existing users, we need to switch to the new ringtones
> immediately so that any incoming calls will use the new sounds. I'm just not
> sure how to do that, if it's even possible.
>
> I don't think storing version info in the ringtones app will help, since we
> should be running this upgrade process even if the user never opens the
> ringtones app.

This sounds like something we should do in runtime before the whole system boot up, if we want to change the system ringtone from the current one to an new one without entering the ringtone app once. I don't know if there is already a mechanism to handle this in gaia, but if not, we should build it then put all the migration things together, and execute them while the system is booting so that the user is able to get the new settings/features after OTA, like the new 2.0 ringtones.

Or this is something that gecko already have?

	Jim Porter (:squib)
	Comment 41 • 10 years ago

Dominic: Yeah, that sounds right. Who should we ask to figure out how to do this?

	Dominic Kuo [:dkuo]
	Comment 42 • 10 years ago

If we want to implement this in gaia, then I believe it should live in system, I will go to Tim or Alive and see if we can do something for this.

	Tiffanie Shakespeare [:tif]
	Comment 43 • 10 years ago

Based on Dominic's comment #42 NI'ing Alive (who appears to be OOO) and Tim.

	Stephany Wilkes
	Comment 44 • 10 years ago

This should have been flagged as 2.0 so I am adding it. There are a lot of other 2.0 tags and flags here but not that one. This was obviously in the Media backlog and being worked on throughout 2.0.

	Hema Koka [:hema]
	Comment 45 • 10 years ago

Swapping existing ringtones with new ones and dealing with migration is way too late for 2.0. We are well past the feature landing date (June 9th) and furthermore, new ringtones will need new labels which will impact L10n as well. We are in string freeze at the moment. 

Based on a quick meeting with Stephany, Tif, Patryk, David and I on 6/27: here is what we plan to do

1. Open a new bug just to add one new Ringtone with Firefox label (no impact to localization since we are using Firefox as the label for this ringtone) --> this bug will be targeted for 2.0 (Patryck to open bug and assign to Jim)

2. Keep this bug for tracking migration work and swaping old ringtones with new ones in the next release 2.1. UX Team: Please keep Tim Chien's team also in the loop for this work.

Thanks
Hema

	Stephany Wilkes
	Comment 46 • 10 years ago

Per Hema and our chat this morning, noting this is now for 2.1 migration work.

See the new bug Hema mentions above for a single new ringtone in bug #1031430.

	Tim Guan-tin Chien [:timdream] (please needinfo)
	Comment 47 • 10 years ago

Hey everyone, there *is* a version number for the whole mozSettings store! Let me explain that in detail, if you haven't read Settings.jsm yourself :)

(I remember asking Gregor to document that somewhere -- but apparently no one need documentation when you have code, <sarcastic>right?</sarcastic>)

(In reply to Tiffanie Shakespeare from comment #43)
> Based on Dominic's comment #42 NI'ing Alive (who appears to be OOO) and Tim.

To recap what I said offline:

User ringtone is currently saved as a blob in mozSettings store under the key "media.ringtone". If there is no user-set ringtone in the user profile, the store will return the default ringtone set on build time, which is swapped during OTA, and the user will hear the new default ringtone. If that's what's needed we are all set, no migration code need to be written -- we should simply swap the media file in the tree and bump the version number for mozSettings store (I am pretty sure we have already done that some point after 1.4). Bumping that number ensures the new default from b2g/defaults/settings.json gets pickup by Gecko.

(In reply to Patryk Adamczyk [:patryk] UX from comment #25)
> Since this is a V2 product, I think its perfectly fine to reset people's
> tones. We will provide a link to download the v.1 tones, but the v.2 tones
> are so much better, I believe most people won't mind.

However, it look like we want to overwrite user-set ringtones during upgrade. I personally don't like the idea because a user-set ringtone might be come from anywhere (e.g. Music app), not just one of the built-in ringtones. Overwrite that might cause the user to lost that data forever, and fail to pick up the first phone call after upgrade. (Flagging :patryk to ensure he realizes the consequence.)

That said, if we really want to do that, we might as well as switch the key for ringtones to "media.ringtone2". An OTA'd user will get a ringtone just like anyone else who get a new phone.

Additionally we may delete the old blob to recover a bit of space if we want to. That would be the only "migration" we need.

	Dominic Kuo [:dkuo]
	Comment 48 • 10 years ago

Thanks Tim for the explanation, and sounds to have a new settings key for the ringtone is an approach we could solve the migration issue, I will file a new bug then let's move these discussions and continue on that bug.

Pavel, would you please update your patch with the test fixed and ask review from Jim again? and after we landed this I think the followup could cover the migration part, thanks.

	Jim Porter (:squib)
	Comment 49 • 10 years ago

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #47)
> However, it look like we want to overwrite user-set ringtones during
> upgrade. I personally don't like the idea because a user-set ringtone might
> be come from anywhere (e.g. Music app), not just one of the built-in
> ringtones. Overwrite that might cause the user to lost that data forever,
> and fail to pick up the first phone call after upgrade. (Flagging :patryk to
> ensure he realizes the consequence.)

Just to note: this only applies to custom ringtone from the music app that was created in version 1.4. In version 2.0, we store copies of *all* custom-created ringtones, even ones that aren't currently set as the default ringer. For custom ringtones created in 2.0, we won't have any data loss.

	Jim Porter (:squib)
	Comment 50 • 10 years ago

Comment on attachment 8435770 [details]
patch for Gaia/master

Most of the stuff for changing the default ringer has moved to bug 1031430, so this PR won't apply to master once that lands (which should be later today).

I think we also probably want all the sounds to be encoded as Opus files. I encoded ringer_firefox.opus with a bitrate of 64kbps, so I think we should do that here as well.

	Tiffanie Shakespeare [:tif]
	Comment 51 • 10 years ago

Hey Jim - would it help at all if you got the raw file of the Firefox ringtone from Patryk for this release?

	Jim Porter (:squib)
	Comment 52 • 10 years ago

Patryk already posted the raw .wav files and I encoded them into Opus.

	Jim Porter (:squib)
	Comment 53 • 10 years ago

Whoops, wrong bug. Yes, that would help.

	Pavel Ivanov [:ivanovpavel][:pivanov] UX Assignee
	Comment 54 • 10 years ago

Hey Jim,
Does that mean this bug is obsolete or we can mark it as duplicate?

	Jim Porter (:squib)
	Comment 55 • 10 years ago

No, we need to add all the *other* ringtones (i.e. not ringer_firefox) in this bug.

	Jim Porter (:squib)
	Comment 56 • 10 years ago

Oh, and sorry for all the confusion. This is a bit convoluted!

	Pavel Ivanov [:ivanovpavel][:pivanov] UX Assignee
	Comment 57 • 10 years ago

no problem :) OK I will rebase this one after bug 1031430 and I will ping you back for r?

	Patryk Adamczyk [:patryk] UX Reporter
	Comment 58 • 10 years ago

Hey Jim, the encoding you did on the firefox ringtone sounds good. So lets keep the same quality throughout. Here are the raw files:
https://mozilla.box.com/s/uc8k5bo0c12g15zh9sl2

Thanks!

	Pavel Ivanov [:ivanovpavel][:pivanov] UX Assignee
	Comment 59 • 10 years ago

Comment on attachment 8435770 [details]
patch for Gaia/master

Hey Jim,
hope everything is ok now

	Jim Porter (:squib)
	Comment 60 • 10 years ago

Comment on attachment 8435770 [details]
patch for Gaia/master

Clearing review for now, since we should convert the sounds to .opus and we also need to discuss what the correct string for ringer_firefox should be.

	Tiffanie Shakespeare [:tif]
	Comment 61 • 10 years ago

I thought it was just Firefox?

	Stephany Wilkes
	Comment 62 • 10 years ago

All, the single ringtone is being tracked here: https://bugzilla.mozilla.org/show_bug.cgi?id=1031430

That's where the use of "Firefox" is noted so as not to have a new string. 

Patryk is on PTO.

	Kai-Chih Hu [:khu]
	Comment 63 • 10 years ago

Given we're in sprint 3 now, can we update the target milestone? If we're comfortable to land this by this month, please change the target milestone to sprint 3. Otherwise, please highlight it, and we should come out some mitigation plans. Thank you.

	Pavel Ivanov [:ivanovpavel][:pivanov] UX Assignee
	Comment 64 • 10 years ago

Close it because of Bug 1046399.

	Jason Smith [:jsmith]
	Comment 65 • 10 years ago

Pavel - All of our tracking flags are currently set on this bug, not bug 1046399. As such, I would prefer that we would keep this bug open for tracking & have the other bug duped this bug, as it will make EPM & QA tracking easier given this is the bug with the feature flag. Does that make sense?

	Pavel Ivanov [:ivanovpavel][:pivanov] UX Assignee
	Comment 66 • 10 years ago

Sure ... my mistake sorry

	Kai-Chih Hu [:khu]
	Comment 67 • 10 years ago

(In reply to Jason Smith [:jsmith] from comment #65)
> Pavel - All of our tracking flags are currently set on this bug, not bug
> 1046399. As such, I would prefer that we would keep this bug open for
> tracking & have the other bug duped this bug, as it will make EPM & QA
> tracking easier given this is the bug with the feature flag. Does that make
> sense?

Thanks, Jason. Sorry, I don't really understand what you mean here. We're expecting all feature-b2g bugs should be closed @ FL. Do we expect this one to be closed at that time as well? This bug looks exactly the same as bug 1046399, and it looks like we're working on the engineering stuff in bug 1046399, but you set tracking flags in this bug, right? If bug 1046399 contains the required engineering stuff for this bug, we may need to configure the dependencies: bug 1014009 depends on bug 1046399, right?

	Jason Smith [:jsmith]
	Comment 68 • 10 years ago

(In reply to Kevin Hu [:khu] from comment #67)
> (In reply to Jason Smith [:jsmith] from comment #65)
> > Pavel - All of our tracking flags are currently set on this bug, not bug
> > 1046399. As such, I would prefer that we would keep this bug open for
> > tracking & have the other bug duped this bug, as it will make EPM & QA
> > tracking easier given this is the bug with the feature flag. Does that make
> > sense?
> 
> Thanks, Jason. Sorry, I don't really understand what you mean here. We're
> expecting all feature-b2g bugs should be closed @ FL. Do we expect this one
> to be closed at that time as well? This bug looks exactly the same as bug
> 1046399, and it looks like we're working on the engineering stuff in bug
> 1046399, but you set tracking flags in this bug, right? If bug 1046399
> contains the required engineering stuff for this bug, we may need to
> configure the dependencies: bug 1014009 depends on bug 1046399, right?

I think we can close this since bug 1046399 landed. Right, this bug has the tracking flags set on it.

	Alison Shiue
	Comment 69 • 10 years ago

Verified landed
Gaia      2be78d83a760fa3b9638fe51c266b442d14597f1
Gecko     https://hg.mozilla.org/mozilla-central/rev/1db35d2c9a2f
BuildID   20140831160203
Version   34.0a1