Closed Bug 1218145 Opened 9 years ago Closed 9 years ago

Add-on Manager should not require DevTools client files

Categories

(Toolkit :: Add-ons Manager, defect)

x86_64
All
defect
Not set
major

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox44 --- affected

People

(Reporter: tonymec, Unassigned)

References

Details

(Keywords: regression, Whiteboard: [seamonkey-2.41-fixed] [fixed by bug 1208112])

User Story

Timestamp: 10/25/2015 10:16:48 AM
Error: 1445764608642	addons.xpi	ERROR	startup failed: [Exception... "Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIXPCComponents_Utils.isModuleLoaded]"  nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)"  location: "JS frame :: resource://gre/modules/addons/XPIProvider.jsm :: XPI_startup :: line 2526"  data: no] Stack trace: XPI_startup()@resource://gre/modules/addons/XPIProvider.jsm:2526 < callProvider()@resource://gre/modules/AddonManager.jsm:221 < _startProvider()@resource://gre/modules/AddonManager.jsm:828 < AMI_startup()@resource://gre/modules/AddonManager.jsm:1006 < AMP_startup()@resource://gre/modules/AddonManager.jsm:2688 < AMC_observe()@resource://gre/components/addonManager.js:58
Source File: resource://gre/modules/Log.jsm
Line: 751
This problem was found on SeaMonkey. I expect it to be platform-independent but I can only test it on linux-x86_64. I don't have Firefox 44.0a1 on this machine.

UA:"Mozilla/5.0 (X11; Linux x86_64; rv:44.0) Gecko/20100101 Firefox/44.0 SeaMonkey/2.41a1" ID:20151024003001 c-c:d583b1409890ee382c1885e03f4ba4020d122dea m-c:26078ba1c7be11cf46706216cad8603b92335cd0 en-US

Reproducible: Always
Steps to Reproduce:
1. Make sure that extensions "Add-ons Manager - Version Number" and/or "Slim Add-ons Manager" are installed and not disabled. Both are restartless, here are their pages at AMO:
https://addons.mozilla.org/addon/amversionnumber/
https://addons.mozilla.org/addon/slim-add-ons-manager/
2. Open the add-ons manager (e.g. browse to about:addons)

Expected results
- Add-ons Manager - Version Number should display a version number next to every add-ons name in the List pane (for any tab except "Get Add-ons")
- Slim Add-ons Manager should display every add-on's description on the same line as its name
- See the pictures on the add-ons' pages for details

Actual results:
They don't. The add-ons manager looks just as if these extensions weren't there.

Additional info:

Last known good:
20151019003001
http://hg.mozilla.org/mozilla-central/rev/d1a89632277fbaaf470c90a35573776048988f2d
http://hg.mozilla.org/comm-central/rev/9579c61e62f5

First known bad:
20151024003001
http://hg.mozilla.org/mozilla-central/rev/26078ba1c7be11cf46706216cad8603b92335cd0
http://hg.mozilla.org/comm-central/rev/d583b1409890

(there were no Linux x86_64 SeaMonkey trunk nightlies between these)

Affects (at least) the following:
Add-ons Manager - Version Number	1.0.6
Slim Add-ons Manager	10.1-signed

The version numbers do appear
- on mouseover
- in the Details pane
- in about:support (Help → Troubleshooting Information)
all of which already exist with no extensions installed.

Both problems could be regarded as "cosmetic" but for me, who have many extensions installed, being able to see as many of them as possible in the add-ons manager (where are the "More" "Release Notes" "Preferences" "Enable/Disable" and "Remove" controls for them) is important. These extensions gave an added value which has somehow vanished.

I know I can see them in about:support but that part of that page is read-only and any action on any add-on would still have to happen in about:addons.

I added the extensions' authors as CC so if the change is intentional (and they can be told what it was) they can update their add-ons. OTOH I don't exclude an unintentional bug as a side-effect of some other change. I also added a few extension developers who know the differences between Firefox and SeaMonkey better than I do.

P.S. I notice a number of recent bugs covering the need (or not) to repack Jetpack add-ons. I can't tell if the above-mentioned restartless add-ons are Jetpack ones but the fact that they used to work on SeaMonkey makes me think that either they aren't, or they had special code to cover the differences between Firefox and SeaMonkey. The following list is thus probably irrelevant, but I cannot tell for sure:
bug 1202902, bug 1212848, bug 1213102, bug 1213160, bug 1217070
P.S. "Lightweight Themes Manager" also doesn't work in the latest nightly. It also worked in the previous one. Symptom: File Not Found

The file chrome://lwthemes/content/lwthemes.xul cannot be found. Please check the location and try again.
P.P.S. "Add-on Update Checker" is not affected. It is also not restartless (I forgot to say that Lightweight Themes Manager is).
Hohoh! Even Adblock Plus (another restartless add-on) is affected. Its button has disappeared from my statusbar, it is also nowhere to be found on the toolbars or the button palette, and clicking its "Preferences" button just gives its about:addons details pane, but with no preferences to be seen, other than "Automatic Updates: (*) Default    ( ) On    ( ) Off    Check for updates"
Severity: normal → major
Summary: Something changed between 19 and 24 october which breaks addons-manager list-pane-related extensions → Something changed between 19 and 24 october which breaks restartless extensions
Yes, and while I'm at it, I checked "Restartless Restart", and sure enough, both its button and menu have also disappeared.

AFAICT, restartless extensions are broken, others aren't.
My money would be on Bug 1203159 and 1212153.

Bug 1203159
Bug 1212153
Bug 1212059

>>
Timestamp: 10/25/2015 10:16:48 AM
Error: 1445764608642	addons.xpi	ERROR	startup failed: [Exception... "Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIXPCComponents_Utils.isModuleLoaded]"  nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)"  location: "JS frame :: resource://gre/modules/addons/XPIProvider.jsm :: XPI_startup :: line 2526"  data: no] Stack trace: XPI_startup()@resource://gre/modules/addons/XPIProvider.jsm:2526 < callProvider()@resource://gre/modules/AddonManager.jsm:221 < _startProvider()@resource://gre/modules/AddonManager.jsm:828 < AMI_startup()@resource://gre/modules/AddonManager.jsm:1006 < AMP_startup()@resource://gre/modules/AddonManager.jsm:2688 < AMC_observe()@resource://gre/components/addonManager.js:58
Source File: resource://gre/modules/Log.jsm
Line: 751
<<
re bug 1212059: Don't know if relevant but the Boolean pref xpinstall.signatures.required defaults to false on SeaMonkey, and I haven't set it to true.
It must have something to do with the installer. Looks like it's missing something. If I start Seamonkey from the dist\bin directory all addons seem to work with VS2013 and VS2015 builds. I just nuked my object directory and started a full build to see if it still works in the dist\bin directory afterwards. 

User agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:44.0) Gecko/20100101 Firefox/44.0 SeaMonkey/2.41a1
Build identifier: 20151024145155
>	NeilAway: https://hg.mozilla.org/mozilla-central/rev/e63ba8ad3c02#l2.12
	- if (Cu.isModuleLoaded("resource:///modules/devtools/ToolboxProcess.jsm")) {
	+ if (Cu.isModuleLoaded("resource:///modules/devtools/client/framework/ToolboxProcess.jsm")) {
	NeilAway: so ToolboxProcess.jsm is now in the client part so you need to apply the patch from Bug 1208112
Depends on: 1208112
(In reply to Frank-Rainer Grahl from comment #7)
> It must have something to do with the installer. Looks like it's missing
> something.
Yes. Try the patch from Bug 1208112
Flags: needinfo?(frgrahl)
Bug 1212153 landed on 9 October, about 10 days before my "last good" build. OTOH, bug 1203159 landed nine commits on 21 October, which is "in range".

Bug 1203159 comment #101 says "We'll likely be fixing this even for apps outside of m-c in bug 1217687" but bug 1217687 landed on October 23 and both the SeaMonkey nightlies from the 24th and the 25th have the problem.

CCing J. Ryan Stinnett to whom both bug 1203159 and bug 1217687 are ASSIGNED. If he didn't cause the failure, maybe he knows who did.
Flags: needinfo?(jryans)
(In reply to Philip Chee from comment #9)
> (In reply to Frank-Rainer Grahl from comment #7)
> > It must have something to do with the installer. Looks like it's missing
> > something.
> Yes. Try the patch from Bug 1208112

I'm on Linux where there is no installer. To update to the next nightly (after downloading the build) I do

rm -Rv /usr/local/seamonkey && tar -jxvf seamonkey-2.41a1.en-US.linux-x86_64.tar.bz2 -C /usr/local || echo 'exit status' $? ; date

i.e. remove the /usr/local/seamonkey directory with all its contents, then recreate it by unpacking the .tar.bz2
IIUC, Frank-Rainer Grahl sees this bug on Windows; this makes it eligible for OS=All.
OS: Linux → All
VS2015 build just finished. Patch seems to be incomplete. Devtools.js is not be found. I will see if I can figure it out.
Flags: needinfo?(frgrahl)
I stand corrected. The first build didn't pick up the config change and just built the server part. Just needed to do a new configure. Addons seems to fully work now. I will test it a little more. I am quite sure this also applies to Linux and just the tar.bz2 generation needs a fix. Do not have a build environment to test.
I just finished testing. My private Windows x64 VS2013 and VS2015 builds work again with the patch Philip provided in Bug 1208112. The patch does not apply clean and needs to be regenerated for the latest trunk. The affected files did change in the meantime.

Thanks
FRG
(In reply to Frank-Rainer Grahl from comment #14)
> I stand corrected. The first build didn't pick up the config change and just
> built the server part. Just needed to do a new configure. Addons seems to
> fully work now. I will test it a little more. I am quite sure this also
> applies to Linux and just the tar.bz2 generation needs a fix. Do not have a
> build environment to test.

I can test on Linux but I don't have a build environment. Too bad it's such a hassle to generate a try-build for SeaMonkey. (IIRC it's barely possible, by including a temporary "patch" to the Thunderbird mozconfig in addition to the SeaMonkey patch to be tested; but it's hardly ever done.)
User Story: (updated)
Depends on: 1212153
Summary: Something changed between 19 and 24 october which breaks restartless extensions → Bug 1212153 breaks restartless extensions in SeaMonkey, Thunderbird ...
UA:"Mozilla/5.0 (X11; Linux x86_64; rv:44.0) Gecko/20100101 Firefox/44.0 SeaMonkey/2.41a1" ID:20151026003002 c-c:b36d991075f18c379bacefe4c408604e417ecc54 m-c:d53a52b39a95dced722cca90ac74529b66dd5253 en-US

Restartless addons are working again. I suspect that this might be due to the fix for bug 1208112 having landed (bug 20151025 comment #20 dated 2015-10-25 22:22:20 PDT).
oops: bug 1208112 comment #20, of course
(In reply to Tony Mechelynck [:tonymec] from comment #17)
> Restartless addons are working again. I suspect that this might be due to
> the fix for bug 1208112 having landed (bug 20151025 comment #20 dated
> 2015-10-25 22:22:20 PDT).
Yup but this isn't a solution for Thunderbird and Instantbird. Why should they have to package the whole devtools client when they can't use any of it?

Probably the devtools team should move ToolboxProcess.jsm to devtools/shared/ or fix Components.utils.isModuleLoaded() to return false instead of throwing NS_ERROR_NOT_AVAILABLE

Lets leave this bug open for Thunderbird, etc.
Here is what I believe happened:

1. Add-on manager has depended on this DevTools client file for quite a while (it probably should not *require* it though)
2. It used to be the case that installing the DevTools client files was triggered by through Firefox's "browser/moz.build".  Perhaps SeaMonkey's build traversed through here, but I am not sure...
3. After both bug 1203159 and the follow up bug 1217687, DevTools client files are only installed if you have set MOZ_DEVTOOLS=all in confvars.sh, which is what was just done for SeaMonkey in bug 1208112.

I believe the correct fix for add-on manager is to allow this DevTools module check to fail without affecting the rest its behavior.

I'll take a look at that.
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Flags: needinfo?(jryans)
Summary: Bug 1212153 breaks restartless extensions in SeaMonkey, Thunderbird ... → Add-on Manager should not require DevTools client files
(In reply to Philip Chee from comment #19)
> (In reply to Tony Mechelynck [:tonymec] from comment #17)
> > Restartless addons are working again. I suspect that this might be due to
> > the fix for bug 1208112 having landed (bug 20151025 comment #20 dated
> > 2015-10-25 22:22:20 PDT).
> Yup but this isn't a solution for Thunderbird and Instantbird. Why should
> they have to package the whole devtools client when they can't use any of it?
> 
> Probably the devtools team should move ToolboxProcess.jsm to
> devtools/shared/ or fix Components.utils.isModuleLoaded() to return false
> instead of throwing NS_ERROR_NOT_AVAILABLE
> 
> Lets leave this bug open for Thunderbird, etc.

isModuleLoaded is at fault here and I can't see what is going on. Is anyone able to debug that call and see where the exception is coming from?
(In reply to Dave Townsend [:mossop] from comment #21)
> isModuleLoaded is at fault here and I can't see what is going on. Is anyone
> able to debug that call and see where the exception is coming from?

The file it's checking for does not exist for some applications, so I expect that is why it fails.

Either isModuleLoaded should return false instead of throwing for non-existent files, or this block should be wrapped in a try since we know it may not exist in some cases.
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #22)
> (In reply to Dave Townsend [:mossop] from comment #21)
> > isModuleLoaded is at fault here and I can't see what is going on. Is anyone
> > able to debug that call and see where the exception is coming from?
> 
> The file it's checking for does not exist for some applications, so I expect
> that is why it fails.
> 
> Either isModuleLoaded should return false instead of throwing for
> non-existent files, or this block should be wrapped in a try since we know
> it may not exist in some cases.

Ah, it does return false for non-existent files but it throws if the url you give it is bad. Do other apps have the resource:/// mapping or do you have to use resource://app/ ?
All apps should now have the resource://devtools/ mapping, as it is installed by /devtools/shared which get pulled into any app that includes /toolkit.

In my testing, it seems things should be functioning correctly.  I tested with Graphene, a B2G desktop derivative, which does not package the DevTools client, so it should be in a similar setup as apps like Thunderbird would.  It returns false when making the same isModuleLoaded call.  Attempting to import the same file throws NS_ERROR_FILE_NOT_FOUND, as expected since the file does not exist.

Philip, is Thunderbird known to be currently hitting this issue, or was that just a guess that it would be in comment 19?
Flags: needinfo?(philip.chee)
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #24)
[...]
> Philip, is Thunderbird known to be currently hitting this issue, or was that
> just a guess that it would be in comment 19?

I don't know (I don't use Thunderbird) but it's easy to test: Install "Add-ons Manager - Version Number" https://addons.mozilla.org/en-US/thunderbird/addon/amversionnumber/ , a restartless extension, into Thunderbird; then have a look at the add-ons manager list pane. Thunderbird is exempt from this issue if and only if you see a version number next to every add-on's name.
P.S. You may have to reload the add-ons manager page.
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #24)

> Philip, is Thunderbird known to be currently hitting this issue, or was that
> just a guess that it would be in comment 19?
I was guessing. Also based on TonyMec reporting the problem and the stack trace.
I'm on Windows and I don't seem to be able to recreate the problem, I'll do another build with DEVTOOLS client turned off
Flags: needinfo?(philip.chee)
(In reply to Tony Mechelynck [:tonymec] from comment #12)
> IIUC, Frank-Rainer Grahl sees this bug on Windows; this makes it eligible
> for OS=All.
I built SeaMonkey locally without the devtools client and restartless addons still work.
Can you guys create a new profile with only *one* addon which should be restartless.
OK I *can* reproduce it.
in installer/package-manifest.in I add a #ifdef:

#ifdef (MOZ_DEVTOOLS)
; DevTools
@RESPATH@/chrome/devtools@JAREXT@
@RESPATH@/chrome/devtools.manifest
@RESPATH@/@PREF_DIR@/devtools.js
#endif

The key is devtools.manifest which contains one line.
resource devtools devtools/modules/devtools/
This comes from devtools/shared/jar.mn

Without this restartless addons fail because there is no resource://devtools/

So my #ifdef should be:

; DevTools
@RESPATH@/chrome/devtools@JAREXT@
@RESPATH@/chrome/devtools.manifest
#ifdef (MOZ_DEVTOOLS)
@RESPATH@/@PREF_DIR@/devtools.js
#endif
Flags: needinfo?(jryans)
Flags: needinfo?(dtownsend)
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #24)
> All apps should now have the resource://devtools/ mapping, as it is
> installed by /devtools/shared which get pulled into any app that includes
> /toolkit.
However each app needs to explicitly package the manifest which defines the resource://devtools/ mapping.
Flags: needinfo?(dtownsend)
(In reply to Philip Chee from comment #30)
> (In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #24)
> > All apps should now have the resource://devtools/ mapping, as it is
> > installed by /devtools/shared which get pulled into any app that includes
> > /toolkit.
> However each app needs to explicitly package the manifest which defines the
> resource://devtools/ mapping.

That's true, the package-manifest.in files are per-app, so they can't be modified centrally to reflect toolkit changes.  However, that's not really specific to this problem, as it could arise with really any change made to Gecko.  (I frequently forget to update package-manifest.in files even inside m-c, so I dislike them in general.)

(In reply to Philip Chee from comment #29)
> OK I *can* reproduce it.
> in installer/package-manifest.in I add a #ifdef:
> 
> #ifdef (MOZ_DEVTOOLS)
> ; DevTools
> @RESPATH@/chrome/devtools@JAREXT@
> @RESPATH@/chrome/devtools.manifest
> @RESPATH@/@PREF_DIR@/devtools.js
> #endif
> 
> The key is devtools.manifest which contains one line.
> resource devtools devtools/modules/devtools/
> This comes from devtools/shared/jar.mn
> 
> Without this restartless addons fail because there is no resource://devtools/
> 
> So my #ifdef should be:
> 
> ; DevTools
> @RESPATH@/chrome/devtools@JAREXT@
> @RESPATH@/chrome/devtools.manifest
> #ifdef (MOZ_DEVTOOLS)
> @RESPATH@/@PREF_DIR@/devtools.js
> #endif

The current code I see in comm-central DXR has no ifdef in suite/installer/package-manifest.in, and the lines are missing entirely from mail/installer/package-manifest.in (but maybe it's outdated).  Some version of the lines should be added to all apps with a package-manifest.in because they all ship the DevTools server by including /toolkit, which needs resource://devtools/ defined.

Either version you quote above *should* function, but you probably need to update installer/Makefile.in to expose the new define to package-manifest.in, or else it would always be undefined (which is likely why your second version currently has different behavior).

You could add all the lines unconditionally, as I believe the package-manifest.in only lists what to package, but does not cause an error if a file it lists is not actually found.  Or if you want to do it conditionally, the jar and manifest file are needed for all values of MOZ_DEVTOOLS, while the prefs are only needed when MOZ_DEVTOOLS=all.

Unassigning for now, as I don't believe there are Gecko changes needed at the moment.
Assignee: jryans → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(jryans)
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WORKSFORME
Whiteboard: [seamonkey-2.41-affected] → [seamonkey-2.41-affected][fixed by bug 1208112]
Whiteboard: [seamonkey-2.41-affected][fixed by bug 1208112] → [seamonkey-2.41-fixed] [fixed by bug 1208112]
You need to log in before you can comment on or make changes to this bug.