[ESR] Some webextension themes with additional backgrounds are not applied in the browser after installation

RESOLVED WONTFIX

Status

defect
RESOLVED WONTFIX
11 months ago
10 months ago

People

(Reporter: amoga, Unassigned)

Tracking

({regressionwindow-wanted})

60 Branch

Firefox Tracking Flags

(firefox-esr60- wontfix, firefox61 unaffected, firefox62 unaffected, firefox63 unaffected)

Details

Attachments

(1 attachment)

[Affected versions]:
 - 60.2.0esr (20180830204350)
 - 60.1.0esr (20180621121604)
 - 60.0 esr (20180503164101)

[Affected Platforms]:
- Windows 10x64
- macOS High Sierra 10.13.2


[Steps to reproduce]:
1. Open an ESR version of Firefox, i.e 60.2.0esr (20180830204350)
2. Go to a multiple backgrounds static theme page and install it (see https://addons-dev.allizom.org/en-US/firefox/addon/alignment-none/?src=search)
3. Check if the theme is applied in the browser


[Expected results]:
Theme is successfully applied in the browser

[Actual results]:
Theme is not applied in the browser, even if its status in about:addons is enabled

Notes:
- other themes that show the same behavior:
https://addons-dev.allizom.org/en-US/firefox/addon/alignment-center/?src=search
https://addons-dev.allizom.org/en-US/firefox/addon/alignment-left-center-right/?src=search
Summary: {ESR} Some webextension themes with additional backgrounds are not applied in the browser after installation → [ESR] Some webextension themes with additional backgrounds are not applied in the browser after installation
Doesn't sound like this is directly related to bug 1443561 since the issue affects all ESR60 releases, but we should still try to get this fixed.

Alexandra, any chance you can track down when this was fixed on trunk with mozregression (using the --find-fix option), assuming it was broken on trunk at some point.
No longer depends on: 1443561
Flags: needinfo?(amoga)
See Also: → 1443561
Hey Ryan,

I've managed to narrow down the regression window to the following changesets: [42ff6656, aa67de9b]

This pointed to a fix introduced with Bug 1465938. 

What is happening now in ESR seems to replicate that issue, meaning that the theme will be finally enabled only after the browser is restarted. 

Hope this helps.
Flags: needinfo?(amoga) → needinfo?(ryanvm)
Thanks, Alexandra. What doesn't make sense to me about that is that AFAICT, that bug should never have affected ESR60. Andrew, do you have any idea what might be going on?
Flags: needinfo?(ryanvm) → needinfo?(aswan)
I'm not sure, bug 1465935 (referenced in comment 3) was cleanup from bug 1461146 which was not part of ESR60.  I suspect the real issue here is in the underlying theme handling and the addons manager bits are just making it harder to see.  Tim, any ideas?
Flags: needinfo?(aswan) → needinfo?(ntim.bugs)
This is the error that's being thrown:

"[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) 
[nsIWebBrowserPersist.saveURI]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: 
resource://gre/modules/LightweightThemeManager.jsm :: _persistImage :: line 872"  data: no]"

Which points to: https://dxr.mozilla.org/mozilla-esr60/source/toolkit/mozapps/extensions/LightweightThemeManager.jsm#872

I'm not quite sure why this is an issue on ESR 60 and not on 62 however.

Also, themes like Weatherlicious also use additional_backgrounds and work fine on ESR 60, so not sure at all what's happening with this specific case.
Dão, I’m not sure if this is right, but I believe this persistImage code can be removed. AFAIK, it was added so we could support file URIs on lightweight themes in a pre-webextension world. Is this correct ?

Even if it is, I’m not sure this is a good candidate for ESR 60 uplift, but my knowledge of saveURI is too limited to provide an immediate fix.
Flags: needinfo?(ntim.bugs)
Flags: needinfo?(dao+bmo)
Any news on this?
Kris, do you have any insight on comment 6 ?
Flags: needinfo?(kmaglione+bmo)
It can't be removed until lightweight themes are retired. They still specify their image resources as AMO CDN URLs, and those need to be cached locally to avoid causing performance issues. To say nothing of the weird prescaling we still do.

It shouldn't be necessary for WebExtension themes, though.
Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(dao+bmo)
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.