Bug 1660557 Comment 14 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

So we have several levels of fail here:

1) The root problem is that at AddonManager startup, we're trying to load previously installed builtin theme that's not there anymore _before_ the call to `maybeInstallBuiltin` in BrowserGlue.

2) The second (minor) problem is that, as I mentioned above, we're _supposed_ to cache the manifest, but we still try to read one from the old location.  This is the error as reported by Richard above.

3) The biggest issue is with the code that's supposed to remedy Problem 1, [by enabling the LWT if it was previously selected](https://searchfox.org/mozilla-central/rev/b2716c233e/toolkit/mozapps/extensions/internal/XPIInstall.jsm#4403-4404,4415,4420).  Unfortunately, it's reading the wrong pref, which was supposed to be fixed in bug 1540856 Part 2, but this instance was missed.

4) With the issue 3 corrected, the active theme is preserved, though not applied on first start after this patch.  This is an improvement obviously (no data loss), but still need to fix this handling, probably in LWT handling code somewhere.

5) Once we fix problem 4, one last potential issue might be a flash of default or incomplete theme on first startup after this patch.  Our current startup setup might not allow us to avoid this without major refactoring or some ugly hard-coding.
So we have several levels of fail here:

1) The root problem is that at AddonManager startup, we're trying to load previously installed builtin theme that's not there anymore _before_ the call to `maybeInstallBuiltin` in BrowserGlue.

2) The second (minor) problem is that, as I mentioned above, we're _supposed_ to cache the manifest, but we still try to read one from the old location.  This is the error as reported by Richard above.  (We can probably track this down, but shouldn't depend on cache being there).

3) The biggest issue is with the code that's supposed to remedy Problem 1, [by enabling the LWT if it was previously selected](https://searchfox.org/mozilla-central/rev/b2716c233e/toolkit/mozapps/extensions/internal/XPIInstall.jsm#4403-4404,4415,4420).  Unfortunately, it's reading the wrong pref, which was supposed to be fixed in bug 1540856 Part 2, but this instance was missed.

4) With the issue 3 corrected, the active theme is preserved, though not applied on first start after this patch.  This is an improvement obviously (no data loss), but still need to fix this handling, probably in LWT handling code somewhere.

5) Once we fix problem 4, one last potential issue might be a flash of default or incomplete theme on first startup after this patch.  Our current startup setup might not allow us to avoid this without major refactoring or some ugly hard-coding.
So we have several levels of fail here:

1) The root problem is that at AddonManager startup, we're trying to load previously installed builtin theme that's not there anymore _before_ the call to `maybeInstallBuiltin` in BrowserGlue.

2) The second (minor) problem is that, as I mentioned above, we're _supposed_ to cache the manifest, but we still try to read one from the old location.  This is the error as reported by Richard above.  (We can probably track this down, but shouldn't depend on cache being there).

3) The biggest issue is with the code that's supposed to remedy Problem 1, [by enabling the LWT if it was previously selected](https://searchfox.org/mozilla-central/rev/b2716c233e/toolkit/mozapps/extensions/internal/XPIInstall.jsm#4403-4404,4415,4420).  Unfortunately, it's reading the wrong pref, which was supposed to be fixed in bug 1540856 Part 2, but this instance was missed.

4) With the issue 3 corrected, the active theme is preserved, though not applied on first start after this patch.  This is an improvement obviously (no data loss), but still need to fix this, probably in LWT handling code somewhere.

5) Once we fix problem 4, one last potential issue might be a flash of default or incomplete theme on first startup after this patch.  Our current startup setup might not allow us to avoid this without major refactoring or some ugly hard-coding.

Back to Bug 1660557 Comment 14