Closed
Bug 1079451
Opened 10 years ago
Closed 10 years ago
Log spew "No valid manifest directive." when uninstalling an add-on
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
Tracking | Status | |
---|---|---|
firefox32 | --- | unaffected |
firefox33 | --- | unaffected |
firefox34 | --- | affected |
firefox35 | --- | affected |
fennec | 34+ | --- |
People
(Reporter: Margaret, Assigned: mossop)
References
Details
(Keywords: regression)
Attachments
(1 file)
1.36 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
I'm seeing this warning fill the log when uninstalling an add-on. Seems to happen with any add-on. [JavaScript Warning: "No valid manifest directive." {file: "jar:jar:file:///data/app/org.mozilla.fennec_leibovic-1.apk!/assets/omni.ja!/components/components.manifest" line: 271}] mfinkle, do you know what this is about? Or where we could look to debug it?
Flags: needinfo?(mark.finkle)
Reporter | ||
Comment 1•10 years ago
|
||
Before all the warnings start, I also see this: I/GeckoConsole(25069): Could not read chrome manifest 'file:///data/data/org.mozilla.fennec_leibovic/chrome.manifest'.
Comment 2•10 years ago
|
||
I don't really know, but... Logged from ParseManifest: http://mxr.mozilla.org/mozilla-central/source/xpcom/components/ManifestParser.cpp#477 Eventually shows up in XPIProvider.jsm as nsJar.getSigningCert in two places: http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/internal/XPIProvider.jsm#1219 http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/internal/XPIProvider.jsm#5274
Flags: needinfo?(mark.finkle)
Reporter | ||
Comment 3•10 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #2) > Eventually shows up in XPIProvider.jsm as nsJar.getSigningCert in two places: > http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/ > internal/XPIProvider.jsm#1219 > http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/ > internal/XPIProvider.jsm#5274 Both of those are from bug 1050518. I wonder if this is a regression.
Reporter | ||
Comment 4•10 years ago
|
||
Hm, this happens on Nightly/Aurora, but not Beta/Release, so I don't think it's a regression from that bug. Mossop, do you know what this could be about?
tracking-fennec: --- → ?
status-firefox32:
--- → unaffected
status-firefox33:
--- → unaffected
status-firefox34:
--- → affected
status-firefox35:
--- → affected
Flags: needinfo?(dtownsend+bugmail)
Keywords: regression
Updated•10 years ago
|
tracking-fennec: ? → 34+
Assignee | ||
Comment 5•10 years ago
|
||
Looks like bug caused this. Affects desktop too.
Blocks: 977026
Component: Add-on Manager → XPCOM
Flags: needinfo?(dtownsend+bugmail) → needinfo?(tlee)
Product: Firefox for Android → Core
Version: Firefox 35 → Trunk
Assignee | ||
Comment 6•10 years ago
|
||
That should have said bug 977026
Assignee | ||
Comment 7•10 years ago
|
||
I'm going to test a patch that backs out the logging from that patch, it looks unnecessary to me.
Assignee: nobody → dtownsend+bugmail
Assignee | ||
Comment 8•10 years ago
|
||
The test for directive->mgrfunc is unnecessary, every directive has either directive->mgrfunc or directive->regfunc. The logging only happens when the directive is for component registration and we are doing chrome-only registration so is not an error.
Attachment #8504142 -
Flags: review?(benjamin)
Comment 9•10 years ago
|
||
Comment on attachment 8504142 [details] [diff] [review] patch rev 1 Since not all directives have a mgrfunc, but we are using ->mgrfunc in this case, what makes this safe instead of potentially introducing a null-deref?
Flags: needinfo?(dtownsend+bugmail)
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #9) > Comment on attachment 8504142 [details] [diff] [review] > patch rev 1 > > Since not all directives have a mgrfunc, but we are using ->mgrfunc in this > case, what makes this safe instead of potentially introducing a null-deref? It's safe because this is the else case from the previous "if (directive->regfunc)": http://mxr.mozilla.org/mozilla-central/source/xpcom/components/ManifestParser.cpp#752 All directives have either a mgrfunc or regfunc so if regfunc is null mgrfunc must not be null.
Flags: needinfo?(dtownsend+bugmail)
Comment 11•10 years ago
|
||
(In reply to Dave Townsend [:mossop] from comment #8) > Created attachment 8504142 [details] [diff] [review] > patch rev 1 This patch seems to fix the issue. My original idea is to print log for every directive not handled. Since it annoys people, we should remove it, and I am sorry for that. But, I suggest to have an assertion to make sure kParsingTable is always correct for debug build.
Flags: needinfo?(tlee)
Updated•10 years ago
|
Attachment #8504142 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 12•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/b008a281364d
Comment 13•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b008a281364d
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•