875854 - [Settings] add missing settings to settingsDB

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect)

Description

[Gregor Wagner [:gwagner]]

There are missing settings in settings.py

Comment 1

[Gregor Wagner [:gwagner]]

Attachment: PR

Comment 2

[Gregor Wagner [:gwagner]]

https://github.com/mozilla-b2g/gaia/commit/95dccd2dbe8e44510ef9f7f3cd006b7720673a16

Comment 3

[Wayne Chang [:wchang]]

Hi Gregor, You're also removing some settings, can we be sure they are not used by anyone else? (Or can no one else use them?)

Triage wants to know if these changes are safe for all other apps.

Thanks.

Comment 4

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Gregor Wagner [:gwagner] from comment #2)
> "ril.sms.requestStatusReport.enabled": False, 

This should be enabled by default, or you'll get several Gecko test case[1] fails.  But, it's true Gecko test cases should never be affected by Gaia changes.  Filed bug 883798 for this.

[1]: https://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/tests/marionette/test_getmessage.js#73 , https://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/tests/marionette/test_outgoing.js#37 , https://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/tests/marionette/test_outgoing_delete.js#38

Comment 5

[Dietrich Ayala (:dietrich)]

No STR, no user impact, no description of which settings are missing, so cannot block on this. If you think we can't wait until 1.2, please renominate with clearer rationale.

Comment 6

[Gregor Wagner [:gwagner]]

(In reply to Wayne Chang [:wchang] from comment #3)
> Hi Gregor, You're also removing some settings, can we be sure they are not
> used by anyone else? (Or can no one else use them?)
> 
> Triage wants to know if these changes are safe for all other apps.
> 
> Thanks.

I just moved them around so they are ordered alphabetically.

Comment 7

[Gregor Wagner [:gwagner]]

(In reply to Dietrich Ayala (:dietrich) from comment #5)
> No STR, no user impact, no description of which settings are missing, so
> cannot block on this. If you think we can't wait until 1.2, please
> renominate with clearer rationale.

Well missing settings can lead to undefined behavior, security risks, crashes... Pick your poison. I don't know if this is actually the case but we made it clear a few times that using the settigs API without a default setting will lead to problems.

Comment 8

[Gene Lian (I already quit Mozilla)]

We need to uplift the |ril.sms.requestStatusReport.enabled| entry to b2g18 to fix bug 887815, comment #8.

Comment 9

[Julien Wajsberg [:julienw]]

And there is near zero chance this can cause problem. Unused settings are unusued anyway, and used settings will be happy to have default values.

Comment 10

[Gene Lian (I already quit Mozilla)]

OK. I see. I can work out another solution on the Gecko part.

Comment 11

[Gregor Wagner [:gwagner]]

I guess Julien said we should take this patch :)

Comment 12

[Gene Lian (I already quit Mozilla)]

OK. Anyway, I provide a patch at Bug 887815 to set |ril.sms.requestStatusReport.enabled| to false by default if it's not available. This way should also work.