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

Status: RESOLVED WONTFIX

(WebExtensions :: Themes, defect)

Description

[Alexandra Moga [:LexaSV]]

[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

Comment 1

[Alexandra Moga [:LexaSV]]

Attachment: esr st backgrounds.gif

Comment 2

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 3

[Alexandra Moga [:LexaSV]]

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.

Comment 4

[Ryan VanderMeulen [: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?

Comment 5

[Andrew Swan [: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?

Comment 6

[Tim Nguyen :ntim]

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.

Comment 7

[Tim Nguyen :ntim]

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.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Any news on this?

Comment 9

[Tim Nguyen :ntim]

Kris, do you have any insight on comment 6 ?

Comment 10

[Kris Maglione [:kmag]]

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.