Closed
Bug 371671
Opened 18 years ago
Closed 17 years ago
Use manifests in Venkman when building with Suiterunner
Categories
(Other Applications Graveyard :: Venkman JS Debugger, defect)
Other Applications Graveyard
Venkman JS Debugger
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: asrail, Assigned: mnyromyr)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
SeaMonkey builds Venkman in its own build and it's moving to toolkits.
Regardless Venkman being built as an extension, it should use manifests instead of contents.rdf for Suiterunner.
It's inside a "#ifdef MOZ_XUL_APP", so it won't break backward compatibility when building it as an extension.
Attachment #256408 -
Flags: review?(silver)
Reporter | ||
Updated•18 years ago
|
Status: NEW → ASSIGNED
Updated•17 years ago
|
Attachment #256408 -
Flags: review?(silver)
Comment 1•17 years ago
|
||
The patch here does not work well for backwards-compat, which venkman wants to preserve though.
We just have realized that we indeed need to come to a stage where venkman (and everything else) needs to have a chrome manifest, or else SeaMonkey does not run when it's installed in a read-only app directory.
Assignee | ||
Comment 2•17 years ago
|
||
This patch makes trunk with enabled venkman extension create a matching chrome.manifest file, thus allowing even for flat or symlinked chrome.
The makexpi.sh workflow shouldn't be affected; the application IDs are taken from install.rdf.
The patch will also fix recent Mac crashes like bug 395552.
(BTW: attachment 256408 [details] [diff] [review] was both bitrotted and wrong.)
Assignee: asrail → mnyromyr
Attachment #256408 -
Attachment is obsolete: true
Attachment #280278 -
Flags: review?
Assignee | ||
Updated•17 years ago
|
Attachment #280278 -
Flags: review? → review?(gijskruitbosch+bugs)
Comment 3•17 years ago
|
||
(In reply to comment #2)
> Created an attachment (id=280278) [details]
> create chrome.manifest for MOZ_XUL_APPs
Depending on what the build system will continue supporting I know they want to drop at least installed-chrome.txt), we could do:
+USE_EXTENSION_MANIFEST = 1
-NO_JAR_AUTO_REG = 1
and drop the jar.mn changes. This would mean the chrome.manifest would be automatically generated from the contents.rdf files and hence make it easier to support. However, like I say the build system may be dropping support for this at some stage.
In either case, we should update extensions/venkman/Makefile.in either have neither variable, or to match the same as the resources/Makefile.in.
Whichever we do, I'd still like to do the update tests I mentioned in bug 392475.
Comment 4•17 years ago
|
||
Dropping the jar.mn changes is no good option as auto-generating manifests does ONLY work if the user running the build has write permissions to the venkman directory - which is NOT the case in many SeaMonkey installs. For those, we need to ship a pre-generated manifest.
As I'm informed by people knowing EM very well, this should work very well as new-toolkit-based apps will ignore the contents.rdf files when a manifest is found and old xpfe-based apps will ignore the manifest and use the contents.rdf instead.
Comment 5•17 years ago
|
||
(In reply to comment #4)
> Dropping the jar.mn changes is no good option as auto-generating manifests does
> ONLY work if the user running the build has write permissions to the venkman
> directory - which is NOT the case in many SeaMonkey installs. For those, we
> need to ship a pre-generated manifest.
Ok, I was wrong earlier, the option I was suggesting doesn't work the way I thought it did.
> As I'm informed by people knowing EM very well, this should work very well as
> new-toolkit-based apps will ignore the contents.rdf files when a manifest is
> found and old xpfe-based apps will ignore the manifest and use the contents.rdf
> instead.
As you say, I expect this will work correctly. However please let me have a bit of time to do the testing I promised Gijs that I would do so that I can confirm it.
Comment 6•17 years ago
|
||
Comment on attachment 280278 [details] [diff] [review]
create chrome.manifest for MOZ_XUL_APPs
Seems like this is our only way out. Yay. :-\
Thanks for the patch though, Mnyromyr. :-)
Attachment #280278 -
Flags: review?(gijskruitbosch+bugs) → review+
Assignee | ||
Comment 7•17 years ago
|
||
Landed attachment 280278 [details] [diff] [review] on trunk.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment 8•17 years ago
|
||
Looks like this fix worked. When I tested the 20070911 trunk nightly, Venkman was back. :)
Based on my experience with bug 395552, I expected one Chrome Registration error and possibly a crash because CZ has not gotten this treatment yet? Instead, SM just started (without CZ.) That's an error that there was no error. Not sure which bug this commentary really belongs on.
Comment 9•17 years ago
|
||
Rich, the chrome error and crash only appear if it's an extension having this problem, and ChatZilla is not build as an extension yet, this will be done in bug 351715
As long as it's no extension, it's just missing but generating no error.
Updated•6 years ago
|
Product: Other Applications → Other Applications Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•