Closed
Bug 952311
Opened 11 years ago
Closed 11 years ago
Update home config when new panels are added/removed
Categories
(Firefox for Android Graveyard :: Awesomescreen, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 30
People
(Reporter: Margaret, Assigned: lucasr)
References
Details
(Whiteboard: shovel-ready)
Bug 862805 added a JS API to let add-ons register new lists to show in about:home, but right now those changes only take effect on restart.
That patch included sending a "HomeLists:Added" message whenever a new list is added, so we can use that to notify the appropriate parties that things should get updated.
That patch did not include anything notification for removing a list, so we'll need to add something to the JS side as well (perhaps another message) to deal with that.
Reporter | ||
Comment 1•11 years ago
|
||
Idea: instead of passing a gecko message, is there a way to listen for changes to shared preferences? One downside to this is that we would need to re-read the preferences and re-load all the lists, but it would be nice to avoid listening for messages from gecko.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → lucasr.at.mozilla
Reporter | ||
Comment 2•11 years ago
|
||
The world has changed since I filed this bug. Once bug 958192 lands, we will keep track of the current set of panels in PanelManager.
To be clear, in this bug I'm talking about what happens when Home.panels.add/remove are called, e.g. when an add-on in installed/uninstalled.
I don't know that we actually need to worry about the case where new panels are added, since the plan is to request to set of available panels whenever the user visits a settings page to edit their panels.
However, when an add-on is uninstalled, we'll need to propagate that change up to the home config and refresh the UI.
Depends on: 958192
Summary: Update home config when new lists are added/removed → Update home config when new panels are added/removed
Reporter | ||
Comment 3•11 years ago
|
||
(In reply to :Margaret Leibovic from comment #2)
> The world has changed since I filed this bug. Once bug 958192 lands, we will
> keep track of the current set of panels in PanelManager.
Actually, this isn't totally correct. We'll use PanelManager to interface with Home.jsm, where we'll actually hold a set of available panels in memory.
Assignee | ||
Comment 4•11 years ago
|
||
For the sake of simplicity, we could do all these updates in the HomeConfig invalidation routine (bug 949174). The following events might trigger an invalidation:
- Locale changes
- An add-on is updated to a new version
- An add-on is uninstalled
The invalidation routine would use the PanelManager to:
- Check if the current panels in HomeConfig are still available
- Re-fetch any layout changes for each panel in HomeConfig
- Re-fetch all strings for each the panel i.e. only the panel title for now
In terms of code, it will probably look a bit like this:
List<String> ids = new ArrayList<String>();
// Populate the ids list from the current list of PanelConfigs in HomeConfig
for (int i = 0; i < panelConfigs.size(); i++) {
ids.add(panelConfigs.get(I).getId());
}
public void requestPanelsById(ids, new RequestCallback() {
@Override
public void onComplete(List<PanelInfo> panelInfos) {
// Recreate list of PanelConfigs based on the returned
// list of PanelInfos. The uninstalled panels will be implicitly
// discarded here. The strings in the PanelInfo instances here
// should be matching the current locale in Gecko.
List<PanelConfig> newPanelConfigs = new ArrayList<PanelConfig>();
for (int i = 0; i < panelInfos.size(); i++) {
PanelConfig panelConfig = convertInfoToConfig(panelInfos.get(i));
newPanelConfigs.add(panelConfig);
}
// Trigger HomeConfig.save() off main thread passing newPanelConfigs().
}
});
Where requestPanelsById() is just a kind of generalization of your requestAvailablePanels() method in bug 958192. The auto-add behaviour will have to go through a different path.
Reporter | ||
Comment 5•11 years ago
|
||
(In reply to Lucas Rocha (:lucasr) from comment #4)
> - An add-on is updated to a new version
> - An add-on is uninstalled
To be clear, we won't actually get events to know the add-on is updated/uninstalled, rather the add-on might add a new panel when it's updated, or it will remove the panel as part of its uninstall handler. Also, we should only need to be updating the HomeConfig when the panels themselves change, not the data inside them (so an add-on syncing new data into its list won't be part of the add-on updating, but rather just something that happens periodically, and querying the content provider will take care of updating that on the UI side). I think we both already know this, but I prefer to err on the side of over-communicating here :)
> The invalidation routine would use the PanelManager to:
> - Check if the current panels in HomeConfig are still available
> - Re-fetch any layout changes for each panel in HomeConfig
> - Re-fetch all strings for each the panel i.e. only the panel title for now
>
> In terms of code, it will probably look a bit like this:
>
> List<String> ids = new ArrayList<String>();
>
> // Populate the ids list from the current list of PanelConfigs in HomeConfig
> for (int i = 0; i < panelConfigs.size(); i++) {
> ids.add(panelConfigs.get(I).getId());
> }
>
> public void requestPanelsById(ids, new RequestCallback() {
> @Override
> public void onComplete(List<PanelInfo> panelInfos) {
> // Recreate list of PanelConfigs based on the returned
> // list of PanelInfos. The uninstalled panels will be implicitly
> // discarded here. The strings in the PanelInfo instances here
> // should be matching the current locale in Gecko.
> List<PanelConfig> newPanelConfigs = new ArrayList<PanelConfig>();
> for (int i = 0; i < panelInfos.size(); i++) {
> PanelConfig panelConfig = convertInfoToConfig(panelInfos.get(i));
> newPanelConfigs.add(panelConfig);
> }
>
> // Trigger HomeConfig.save() off main thread passing
> newPanelConfigs().
> }
> });
>
> Where requestPanelsById() is just a kind of generalization of your
> requestAvailablePanels() method in bug 958192. The auto-add behaviour will
> have to go through a different path.
This sounds good. So, given this, we won't need the "HomePanels:Remove" message, but rather we will trigger something to invalidate the HomeConfig when the set of available panels changes?
Assignee | ||
Comment 6•11 years ago
|
||
(In reply to :Margaret Leibovic from comment #5)
> This sounds good. So, given this, we won't need the "HomePanels:Remove"
> message, but rather we will trigger something to invalidate the HomeConfig
> when the set of available panels changes?
Pretty much. But we'll have to be a bit smart when handling add-on updates. IIURC, add-ons will cleanup their current panels and then re-add them when a new version is installed?
We'll have to:
1. Avoid triggering a HomeConfig invalidation if the panels are being removed as part of an add-on update (otherwise they'll be unintentionally dropped from HomeConfig).
2. Avoid triggering two HomeConfig invalidations when add-ons are updates. One for when the panels are removed and another for when the panels are re-added.
Reporter | ||
Comment 7•11 years ago
|
||
(In reply to Lucas Rocha (:lucasr) from comment #6)
> (In reply to :Margaret Leibovic from comment #5)
> > This sounds good. So, given this, we won't need the "HomePanels:Remove"
> > message, but rather we will trigger something to invalidate the HomeConfig
> > when the set of available panels changes?
>
> Pretty much. But we'll have to be a bit smart when handling add-on updates.
> IIURC, add-ons will cleanup their current panels and then re-add them when a
> new version is installed?
I'm not sure exactly how updates work, but I imagine when restartless add-ons are updated, this is basically what happens. And remember, the way restartless add-ons work is they're basically "installed" on every startup, which makes things like persistent state between app runs (like a persistent panel in the UI) tricky :/
Assignee | ||
Comment 8•11 years ago
|
||
(In reply to :Margaret Leibovic from comment #7)
> (In reply to Lucas Rocha (:lucasr) from comment #6)
> > (In reply to :Margaret Leibovic from comment #5)
> > > This sounds good. So, given this, we won't need the "HomePanels:Remove"
> > > message, but rather we will trigger something to invalidate the HomeConfig
> > > when the set of available panels changes?
> >
> > Pretty much. But we'll have to be a bit smart when handling add-on updates.
> > IIURC, add-ons will cleanup their current panels and then re-add them when a
> > new version is installed?
>
> I'm not sure exactly how updates work, but I imagine when restartless
> add-ons are updated, this is basically what happens. And remember, the way
> restartless add-ons work is they're basically "installed" on every startup,
> which makes things like persistent state between app runs (like a persistent
> panel in the UI) tricky :/
We should use the 'reason' argument that bootstrapped add-ons take to give enough context to figure out what to do with each add()/remove() call. I filed bug 968188 to track this.
Assignee | ||
Comment 9•11 years ago
|
||
Patches for bug 964375 fixes this bug.
https://hg.mozilla.org/integration/fx-team/rev/4103c332f013
https://hg.mozilla.org/integration/fx-team/rev/31f1a0b3b5b8
https://hg.mozilla.org/integration/fx-team/rev/a49670361d8a
https://hg.mozilla.org/integration/fx-team/rev/3ce5275b17ed
https://hg.mozilla.org/integration/fx-team/rev/299bcc7ae1fe
https://hg.mozilla.org/integration/fx-team/rev/cfd371a2c63b
https://hg.mozilla.org/integration/fx-team/rev/1a5908740d35
https://hg.mozilla.org/integration/fx-team/rev/9385b83f32c5
https://hg.mozilla.org/integration/fx-team/rev/75b2985c88bb
https://hg.mozilla.org/integration/fx-team/rev/f9977589e824
https://hg.mozilla.org/integration/fx-team/rev/9897f4f42176
Depends on: 964375
Assignee | ||
Comment 10•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4103c332f013
https://hg.mozilla.org/mozilla-central/rev/31f1a0b3b5b8
https://hg.mozilla.org/mozilla-central/rev/a49670361d8a
https://hg.mozilla.org/mozilla-central/rev/3ce5275b17ed
https://hg.mozilla.org/mozilla-central/rev/299bcc7ae1fe
https://hg.mozilla.org/mozilla-central/rev/cfd371a2c63b
https://hg.mozilla.org/mozilla-central/rev/1a5908740d35
https://hg.mozilla.org/mozilla-central/rev/9385b83f32c5
https://hg.mozilla.org/mozilla-central/rev/75b2985c88bb
https://hg.mozilla.org/mozilla-central/rev/f9977589e824
https://hg.mozilla.org/mozilla-central/rev/9897f4f42176
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•