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

RESOLVED WONTFIX

Status

defect
RESOLVED WONTFIX
8 months ago
7 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)

(Reporter)

Description

8 months ago
[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
(Reporter)

Comment 1

8 months ago
(Reporter)

Updated

8 months ago
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
(Reporter)

Comment 3

8 months ago
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.
(Reporter)

Updated

8 months ago
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)

Comment 6

8 months ago
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

8 months ago
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)

Updated

8 months ago
Flags: needinfo?(dao+bmo)
Any news on this?

Comment 9

7 months ago
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
Last Resolved: 7 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.