The panel options hold l10n material that should be refreshed if the locale changes. In practice, the list of panels returned from requestAvailablePanels and requestPanelsById should be dynamically generated instead of using a fixed list of registered panels. This will ensure that the panels returned from these calls are always using strings for the current locale. Initial API idea: register(panelID, function() { return PANEL_OPTIONS; }); unregister(panelID); Let's first redefine the panels API in bug 972306 and then work on this.

Comment on attachment 8380712 [details] [diff] [review] Redefine registration logic to return panel options dynamically (r=margaret) Here's an initial proposal. This changes the panel registry to generate options dynamically as described in the bug's description: let optionsCallback = function() { return Object.freeze({ title: gStrings.GetStringFromName("title"), layout: Home.panels.Layout.FRAME, views: [ { type: Home.panels.View.GRID, dataset: DATASET_ID } ] }); }; Home.panels.register("panel-id", optionsCallback); Note that the returned options object doesn't need to contain the panel's ID anymore. This is to avoid confusion by having one ID passed to the register() call and another that is returned by the options callback. Locale changes are handled correctly with this new API. With this patch, we generate Panel instances on demand. This patch introduces some overhead from validating every Panel instance after creation (especially when fetching available panels). But we can easily optimize that in future (e.g. only refresh instances when the locale changes) without breaking the API. I like that.

Comment on attachment 8380712 [details] [diff] [review] Redefine registration logic to return panel options dynamically (r=margaret) Review of attachment 8380712 [details] [diff] [review]: ----------------------------------------------------------------- This is looking good, but I have some comments about how we could make things even better. Feel free to re-request review if you make an updated patch that incorporates any of these ideas. ::: mobile/android/modules/Home.jsm @@ +172,5 @@ > let HomePanels = (function () { > // Holds the current set of registered panels that can be > // installed, updated, uninstalled, or unregistered. This > // is also used to retrieve the list of available panels > // in the system (see HomePanels:Get handler). Update this comment to mention this maps to functions we use to dynamically generate the panels. @@ +199,5 @@ > + INTENT: "intent" > + }); > + > + let _createAndValidatePanel = function(id, optionsCallback) { > + let panel = new Panel(id, optionsCallback()); Perhaps _createAndValidatePanel should just take an id, and it can get the optionsCallback itself out of _registeredPanels. It can also throw an exception if it doesn't actually find a registered panel for that id, since right now we would probably just get some error like "undefined isn't a function" if that were the case. @@ +249,5 @@ > id: panel.id, > title: panel.title, > layout: panel.layout, > views: panel.views > }; This code has evolved over time, so maybe at one point this was useful, but now it seems like we could just return the panel object itself here. However, if we're going to do that, we probably need to make sure to always set all the properties to something in the Panel constructor (or ensure our Java consumers would be okay with missing properties). Also, since this is the only place we use _createAndValidatePanel, perhaps we can get rid of _panelToJSON and just replace its use with _createAndValidatePanel. _createAndValidatePanel also seems a bit clunky, maybe a better name could be something like _generatePanel. I don't have a strong opinion here, though. @@ +305,3 @@ > } > > + _registeredPanels[id] = optionsCallback; I think we should also throw if optionsCallback isn't a function.

Comment on attachment 8381331 [details] [diff] [review] Redefine registration logic to return panel options dynamically (r=margaret) Covers review suggestions.

Comment on attachment 8381331 [details] [diff] [review] Redefine registration logic to return panel options dynamically (r=margaret) Review of attachment 8381331 [details] [diff] [review]: ----------------------------------------------------------------- Nice. ::: mobile/android/modules/Home.jsm @@ +161,2 @@ > > if ("title" in options) Since we require that a panel has a title as well, we could also get rid of this check.

Setting P1 hub bugs to block hub v1 EPIC bug (targeting fx30 release). Filter on epic-hub-bugs.