916718 - Check the event customization type before remove the Wallpaper customization listener

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::First Time Experience, defect)

Description

[Marina Rodríguez [:mai]]

Now, wallpaperCustomizer removes the customization listener without checking if it's the owner of the event. In this way, it only works if the first received event is a wallpaper customization event.
Added unit test to prevent the same mistakes to occur again.

Comment 1

[Jason Smith [:jsmith]]

What's the user impact of this? Does this need to block?

Comment 2

[Marina Rodríguez [:mai]]

Attachment: patch v1 - Fix error removing listener

Fix error and added unit test.

Comment 3

[Marina Rodríguez [:mai]]

For the user, it is completely transparent. The problem is for the OEM, because if the first launched customization event is not a wallpaper customization, then the wallpaper will be the default one.

Comment 4

[Jason Smith [:jsmith]]

(In reply to marina rodríguez [:mai] from comment #3)
> For the user, it is completely transparent. The problem is for the OEM,
> because if the first launched customization event is not a wallpaper
> customization, then the wallpaper will be the default one.

So the impact here is that the wallpaper customization won't work if this happens then, right? What's the probability of this happening?

Comment 5

[Borja Salguero [:borjasalguero]]

After trying the code I've detected several issues, some of them not related with this patch, more related about how we are dealing with Customization params.
When trying with my SIM Card, I got the following error:

E/GeckoConsole(  626): [JavaScript Error: "NS_ERROR_FILE_NOT_FOUND: " {file: "app://communications.gaiamobile.org/ftu/js/variant.js" line: 45}]

This is due to the file, generated following the path:
var filePath = '/ftu/js/variants/' + self.mcc_mnc + '.json';

is not generating a 404 error due to https://bugzilla.mozilla.org/show_bug.cgi?id=827243, so we would need to add a try...catch for handling this properly.

I would suggest to create a file called 'customization.json', with an array of objects with pair MCC/MNC, and with the params to customize (wallpaper, ringtone...). All resources should be *out* of this file (we need only the URI of the resource, not the resource itself in Base64), and the app will be in charge of retrieving these resources and make all the process needed. Having files embedded in Base64 in the .json, and creating one file per MNC/MCC is a more complicated solution IMHO.

Reuben, I will try to talk with you through IRC today, but wdyt about this proposal? Do you think we could move forward with this new approach? Thanks!

Comment 6

[Reuben Morais [:reuben]]

Forwarding to Francisco since he objected to that design.

Comment 7

[Francisco Jordano [:arcturus] [:francisco]]

Hi folks,

as Reuben commented, I don't like the idea of having a single file cause we will be loading a (potentially) huge file to just strip a couple of fields.

My first approach to make this dependant on bug 827243, or follow up with the workaround with the try/catch as Borja comments.

Cheers,
F.

Comment 8

[Borja Salguero [:borjasalguero]]

Fixed in bug https://bugzilla.mozilla.org/show_bug.cgi?id=917740

Comment 9

[Borja Salguero [:borjasalguero]]

Comment on attachment 805344 [details]
patch v1 - Fix error removing listener

THis was fixed by https://bugzilla.mozilla.org/show_bug.cgi?id=917740