1000860 - Clean up bootstrap mozL10n API use in Setringtone app

Status: RESOLVED INVALID

(Firefox OS Graveyard :: Gaia, defect)

Description

[Staś Małolepszy :stas]

Not sure where this should go into (Music? Settings?).  Filing in Gaia , please move as appropriate.

The Setringtone app currently uses a combination of window.onload and mozL10n.ready to work around the non-deterministic mozL10n.ready behavior.  Bug 993188 has the fix and bug 993189 introduced mozL10n.once, which is better-suited for initialization tasks.

Comment 1

[Staś Małolepszy :stas]

Attachment: Use mozL10n.once

https://github.com/mozilla-b2g/gaia/pull/18642

Comment 2

[David Flanagan [:djf]]

Stas,

I can't figure out whether the once() callback is guaranteed to be invoked after the document load event.  If so, then this patch is fine, but if not, then this patch might change the startup behavior.  If you clarify how once() works, I'll finish the review of this and the related bugs.

Comment 3

[Staś Małolepszy :stas]

Comment on attachment 8411767 [details] [diff] [review]
Use mozL10n.once

David, your concerns are valid: once() is indeed not guaranteed to wait for load or DOMContentLoaded.   I've been actually working on this in bug 1000806, and I'm sorry for not canceling the review request while this remains an open issue.

I can either a) bring window.onload back in the patch or b) wait for bug 1000806 to define the exact strategy for this kind of cases.  I suggest we wait (option b) since the fact that we're calling ready here instead of once will only cause a bug when the current language changes, which is rare.

Are you okay with this?

Comment 4

[David Flanagan [:djf]]

Yes, let's just wait on this. Since the set ringtone app is only involved via activity, it is probably never actually possible for it to experience a language change, anyway.

Heads up that lots of ringtone changes will be landing on master soon, so your patch may need to be rewritten anyway.

Comment 5

[Staś Małolepszy :stas]

(In reply to David Flanagan [:djf] from comment #4)
> Heads up that lots of ringtone changes will be landing on master soon, so
> your patch may need to be rewritten anyway.

David, it looks like the Setringtone app was rewritten and integrated into apps/ringtones in bug 960329.  It now correctly uses mozL10n.once, so I suggest we close this bug as worksforme or invalid.  Do you agree?

Comment 6

[David Flanagan [:djf]]

I agree.  Closing the bug.