1135251 - [Gallery] ThumbnailList class should not call navigator.mozL10n.ready()

Status: RESOLVED FIXED

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

Description

[David Flanagan [:djf]]

gallery/js/thumbnail_list.js calls mozL10n.ready() in its constructor. Currently gallery only ever creates a single ThumbnailList with the same lifetime as the app, so this is okay.  But if we ever modified it to create multiple ThumbnailList objects, then this would be a tricky memory leak.

So we should change this. I suggest modifying ThumbnailList to use once() for its initial localization.  Then, once Gallery has created its thumbnail list, it can call ready() at the global level to invoke thumbnailList.localize() when the language changes.

See https://bugzilla.mozilla.org/show_bug.cgi?id=1068976#c7 and https://bugzilla.mozilla.org/show_bug.cgi?id=1134436#c6 for more details.

Comment 1

[David Flanagan [:djf]]

Punam,

Would you take this bug, please?

Comment 2

[Punam Dahiya [:pdahiya]]

Thanks for filing, assigning myself to fix after updating Bug 1134436 patch.

Comment 3

[GitHub Autolander Bot]

Attachment: [gaia] punamdahiya:Bug1135251 > mozilla-b2g:master

Comment 4

[Punam Dahiya [:pdahiya]]

Comment on attachment 8568690 [details] [review]
[gaia] punamdahiya:Bug1135251 > mozilla-b2g:master

Hi David
Please review attached PR updated with ThumbnailList to use once for its initial localization and ready call moved to global level inside startup.js after thumbnails are created. Thanks!

Comment 5

[David Flanagan [:djf]]

Comment on attachment 8568690 [details] [review]
[gaia] punamdahiya:Bug1135251 > mozilla-b2g:master

r- because I think this patch can cause the list to be localized twice on startup. If you just do the ready() call right after calling the ThumbnailList constructor (with nothing async in between) then I think that is all that is needed. See my comment on github.

Comment 6

[Punam Dahiya [:pdahiya]]

Comment on attachment 8568690 [details] [review]
[gaia] punamdahiya:Bug1135251 > mozilla-b2g:master

Hi David
I have updated patch with your review feedback. You are right we don't need explicit l10n once call in ThumbnailList constructor if we move  ready() call right after creating thumbnailList instance. Thanks

Comment 7

[David Flanagan [:djf]]

Comment on attachment 8568690 [details] [review]
[gaia] punamdahiya:Bug1135251 > mozilla-b2g:master

Looks good to me!

Comment 8

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/27814b20ca2bf33c040f86ee81aa34b562f01150