Closed Bug 1487631 Opened 2 years ago Closed 2 years ago

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

Categories

(WebExtensions :: Themes, defect)

60 Branch
defect
Not set
normal

Tracking

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

RESOLVED WONTFIX
Tracking Status
firefox-esr60 - wontfix
firefox61 --- unaffected
firefox62 --- unaffected
firefox63 --- unaffected

People

(Reporter: LexaSV, Unassigned)

References

Details

(Keywords: regressionwindow-wanted)

Attachments

(1 file)

[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
Attached image esr st backgrounds.gif
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: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.