Closed
Bug 564667
Opened 14 years ago
Closed 13 years ago
Allow bootstrapped add-ons to have chrome
Categories
(Toolkit :: Add-ons Manager, enhancement)
Toolkit
Add-ons Manager
Tracking
()
RESOLVED
FIXED
mozilla8
People
(Reporter: jwkbugzilla, Assigned: peregrino)
References
Details
(Keywords: dev-doc-complete, Whiteboard: [rewrite])
Attachments
(1 file, 5 obsolete files)
23.40 KB,
patch
|
mossop
:
superreview+
|
Details | Diff | Splinter Review |
As it is now, bootstrapped add-ons cannot have their own chrome namespace, this makes implementing a dialog for example unnecessarily complicated (or simply making existing extensions bootstrapped). While I see why overlays might be a problem, a chrome namespace certainly shouldn't be. I see three approaches how this could be implemented: 1. Don't ignore chrome.manifest of bootstrapped add-ons (might be complicated the way it is currently processed). 2. Add API to nsIChromeRegistry to trigger processing of a chrome.manifest file manually (probably also to remove entries of a chrome.manifest file though this isn't absolutely necessary). 3. Add API to nsIChromeRegistry to allow manual manipulation of chrome namespaces so that extensions can register and unregister namespaces without using chrome.manifest.
Updated•14 years ago
|
Whiteboard: [rewrite]
Comment 1•14 years ago
|
||
Do you really need "chrome"? would some other resource namespace work as well?
Comment 2•14 years ago
|
||
One of the biggest benefits of the chrome namespace as registered through chrome.manifest is the ease of adding localization. Add one line and: chrome://<namespace>/locale/<filename> just magically works with no extra effort. There really isn't any other way to easily set things up to automatically fetch the needed file in the current locale and this is what was built to do that, so there's no reason it shouldn't be used for bootstrapped addons too. Without this, localization is impractical.
Depends on: 628089
Comment 3•13 years ago
|
||
this can be implemented easily with js only addonManager should register some dummy manifest on sturtup and when new bootstraped addons are installed append 'content' and other declarations from it's manifest into dummy manifest changing relative paths to absolute and call nsIChromeRegistry.checkForNewChrome() this little hack can save lot of hack-o-meters, until c++ implementation is ready could this possibly be in 4 or it's too late already?
Comment 4•13 years ago
|
||
(In reply to comment #3) > this can be implemented easily with js only > addonManager should register some dummy manifest on sturtup > and when new bootstraped addons are installed > append 'content' and other declarations from it's manifest into dummy manifest > changing relative paths to absolute > and call nsIChromeRegistry.checkForNewChrome() > > this little hack can save lot of hack-o-meters, until c++ implementation is > ready > could this possibly be in 4 or it's too late already? This wouldn't work since the chrome manifests are only read on startup. When a bootstrapped add-on was installed a restart would be necessary to make its chrome work in your suggestion which negates the point of restartless add-ons.
Comment 5•13 years ago
|
||
new chrome manifests are registered on startup only but changes in installed manifests are easily picked by gChromeReg = Cc["@mozilla.org/chrome/chrome-registry;1"].getService(Ci.nsIXULChromeRegistry); gChromeReg.checkForNewChrome() so if we add content bootchrome file:///absolute/path/to/folder/ to one of already registered manifests and call checkForNewChrome chrome://bootchrome/content/ will point to folder
Comment 6•13 years ago
|
||
to demonstrate what i mean install this addon into profile and then from options dialog you can add any folder with any chrome alias
(In reply to comment #1) > Do you really need "chrome"? would some other resource namespace work as well? In Firefox 4, only chrome:// uris can use XUL (without playing with the remote XUL whitelist extension thing). Last I checked, resource:// uris using XUL trigger the "No remote XUL" error page (and that remote XUL whitelist extension doesn't work for resource uris, if I recall correctly). All of this is gone over in more detail in bug 628089. This usecase wouldn't necessarily require "chrome://", but would need some almost-equivalent "privileged" namespace that could avoid the "no remote XUL" restrictions.
Updated•13 years ago
|
Assignee: nobody → dtownsend
Reporter | ||
Comment 8•13 years ago
|
||
(In reply to comment #7) > In Firefox 4, only chrome:// uris can use XUL (without playing with the remote > XUL whitelist extension thing). Last I checked, resource:// uris using XUL > trigger the "No remote XUL" error page (and that remote XUL whitelist extension > doesn't work for resource uris, if I recall correctly). That's not quite correct - any protocol using the system principal can use XUL. resource:// protocol doesn't change privileges so if you have a mapping from resource:// to file:// then you will get the privileges of local files - not good enough for XUL. However, with a custom protocol you can have XUL. The reason for this bug is that a custom protocol handler isn't exactly something you want reimplemented in every extension - and some things like DTDs really require chrome:// and won't accept anything else.
Comment 9•13 years ago
|
||
(In reply to comment #0) > 2. Add API to nsIChromeRegistry to trigger processing of a chrome.manifest file > manually (probably also to remove entries of a chrome.manifest file though this > isn't absolutely necessary). I'd prefer this option. In Lightning, we have a longstanding issue that executing unit tests does not work since starting Thunderbird's xpcshell doesn't cause extension's chrome.manifests to be parsed. Of course we could hack it in by temporarily adding include extensions/uuid/chrome.manifest to the TB chrome.manifest, but if this bug was fixed by option 2, then we could just add it to a head_foo.js file and be done with it. This might also be handy for other extensions that use the build system.
Comment 10•13 years ago
|
||
(In reply to comment #3) > addonManager should register some dummy manifest on sturtup > and when new bootstraped addons are installed > append 'content' and other declarations from it's manifest into dummy manifest If you want to hack in a chrome manifest file, you don't need to get that hackish. nsIComponentRegistrar::autoRegister can register new chrome.manifest files, and yes, it does work to register a manifest dynamically (so chrome manifest files are not limited to being read only on startup). There are two problems with particular hack: 1) There is no way to unregister that chrome, and additionally, since chrome is cached, any changes (from a new version of the addon) won't take effect unless you also flush the entire chrome cache (ick!). 2) Requires em:unpack=true (IIRC). Bottom line is, there isn't a suitable way to do this that isn't hackish (and that won't result in an AMO review denial). PS: If support for creating chrome:// URLs is indeed added, then there needs to be a way to version-check em:boostrapped. If an addon requires the use of a chrome URL, then it needs a way to signal to the browser that it is bootstrappable in Firefox 5 (or whatever) but that it should use the old non-bootstrapped install for Firefox 4 and older.
Comment 11•13 years ago
|
||
(In reply to comment #10) > nsIComponentRegistrar::autoRegister can register new chrome.manifest files, and > yes, it does work to register a manifest dynamically (so chrome manifest files > are not limited to being read only on startup). Great, this is totally sufficient for Lightning's needs, of course that doesn't touch the actual bug issue. Sorry for the noise and thanks for the hint!
Updated•13 years ago
|
Assignee: dtownsend → nobody
Assignee | ||
Comment 13•13 years ago
|
||
Proposed patch for this feature. This patch adds the following methods to the ComponentManager: XRE_AddBootstrappedManifestLocation(nsILocalFile* aLocation) XRE_RemoveBootstrappedManifestLocation(nsILocalFile* aLocation) That add and remove chrome manifests on runtime. Then a bootstrapped addon should only need to add this code so that a chrome.manifest file bundled with the addon is loaded: function startup (params, aReason) { Components.manager.XRE_AddBootstrappedManifestLocation(params.installPath); } function shutdown (params, aReason) { Components.manager.XRE_RemoveBootstrappedManifestLocation(params.installPath); }
Attachment #544644 -
Flags: review?(dtownsend)
Comment 14•13 years ago
|
||
Could we do this automatically on startup and shutdown? It seems unnecessary to add it to the bootstrap methods.
The call to CheckForNewChrome raises a number of warnings on the Error Console, e.g.:
> Warning: Duplicate resource declaration for 'services-sync' ignored.
> Source File: jar:file:///c:/users/geoff/firefox/omni.jar!/components/components.manifest
> Line: 293
We should create a way of just removing the manifest from the chrome registry.
Also, I suspect you're going to have problems with caching, e.g. if a newer version of the addon is installed.
Comment 15•13 years ago
|
||
(In reply to comment #14) > Could we do this automatically on startup and shutdown? It seems unnecessary > to add it to the bootstrap methods. Yeah I think we might end up doing this, but getting the capability there is an awesome checkpoint. > The call to CheckForNewChrome raises a number of warnings on the Error > Console, e.g.: > > Warning: Duplicate resource declaration for 'services-sync' ignored. > > Source File: jar:file:///c:/users/geoff/firefox/omni.jar!/components/components.manifest > > Line: 293 > We should create a way of just removing the manifest from the chrome > registry. I'm not sure these warnings are a big deal and could pretty easily just suppress them. That's almost certainly easier than the work required to remove stuff from the registry selectively. > Also, I suspect you're going to have problems with caching, e.g. if a newer > version of the addon is installed. We blow away all the caches when you uninstall/upgrade add-ons so I don't think there is an issue here.
Comment 16•13 years ago
|
||
(In reply to comment #15) > (In reply to comment #14) > > Also, I suspect you're going to have problems with caching, e.g. if a newer > > version of the addon is installed. > > We blow away all the caches when you uninstall/upgrade add-ons so I don't > think there is an issue here. Ah, I see that's changed since I was mucking around with this stuff 6 months ago.
Comment 17•13 years ago
|
||
Comment on attachment 544644 [details] [diff] [review] Proposed Patch Review of attachment 544644 [details] [diff] [review]: ----------------------------------------------------------------- This is really close but I'd like to spin over it again before I pass it over to bsmedberg. Can you make a second test that does the same as the first but where the manifest is inside an xpi file. ::: chrome/test/unit/data/test_bug564667/loaded.manifest @@ +1,2 @@ > +content test2 test/ > +locale test2 en-US test/ What is this file for? ::: chrome/test/unit/test_bug564667.js @@ +13,5 @@ > + * > + * The Original Code is mozilla.org code. > + * > + * The Initial Developer of the Original Code is > + * Hernan Rodriguez Colmeiro <colmeiro@gmail.com>. As you're working for us the initial developer is "the Mozilla Foundation.". You can put yourself as a contributor @@ +15,5 @@ > + * > + * The Initial Developer of the Original Code is > + * Hernan Rodriguez Colmeiro <colmeiro@gmail.com>. > + * > + * Portions created by the Initial Developer are Copyright (C) 2007 I could be wrong but I think it is 2011 @@ +38,5 @@ > + > +const MANIFEST_PATH = do_get_file("data/test_bug564667"); > + > +var gIOS = Cc["@mozilla.org/network/io-service;1"]. > + getService(Ci.nsIIOService); Alight the getService with the Cc above, same for the others below @@ +44,5 @@ > +var gCR = Cc["@mozilla.org/chrome/chrome-registry;1"]. > + getService(Ci.nsIChromeRegistry); > + > +var gOP = Cc["@mozilla.org/chrome/chrome-registry;1"]. > + getService(Ci.nsIXULOverlayProvider); If you just QueryInterface gCR to nsIXULOverlayProvider then you'll be able to use the same variable for both @@ +46,5 @@ > + > +var gOP = Cc["@mozilla.org/chrome/chrome-registry;1"]. > + getService(Ci.nsIXULOverlayProvider); > + > +var baseURI = gIOS.newFileURI(do_get_file("data/test_bug564667")).spec; Use MANIFEST_PATH here. @@ +59,5 @@ > + var result = gCR.convertChromeURL(uri); > + do_check_eq(result.spec, target); > + } > + catch (ex) { > + do_throw(chromeURL+" not Registered"); Nit: put spaces around operators like + @@ +70,5 @@ > +function test_removed_mapping(chromeURL, target) { > + var uri = gIOS.newURI(chromeURL, null, null); > + try { > + var result = gCR.convertChromeURL(uri); > + do_throw(chromeURL+" not removed"); Same again @@ +92,5 @@ > + var overlays = (type == "overlay") ? > + gOP.getXULOverlays(uri) : gOP.getStyleOverlays(uri); > + > + // We shouldn't be allowed to register overlays or styles > + while( overlays.hasMoreElements() ) { Nit: Space between while and the open bracket, no spaces immediately inside the brackets. @@ +94,5 @@ > + > + // We shouldn't be allowed to register overlays or styles > + while( overlays.hasMoreElements() ) { > + elem = overlays.getNext().QueryInterface(Ci.nsIURI); > + present = present || (elem.spec == target); You should just test elem.spec == target and do_throw if it is, no need for the present tracking I think. Or perhaps even easier, overlay some made up url instead of browser.xul and then you only need to test if overlays.hasMoreElements(). If it does then it is a failure. @@ +97,5 @@ > + elem = overlays.getNext().QueryInterface(Ci.nsIURI); > + present = present || (elem.spec == target); > + } > + > + if(present) { Nit: Space between if and the open bracket. @@ +98,5 @@ > + present = present || (elem.spec == target); > + } > + > + if(present) { > + if(type == "style") And again @@ +110,5 @@ > +{ > + //Add a manifest file > + Components.manager.XRE_AddBootstrappedManifestLocation(MANIFEST_PATH); > + > + // Test Adding Content URL Indentation here is a little messed up ::: xpcom/components/ManifestParser.cpp @@ +75,5 @@ > bool componentonly; > > bool ischrome; > > + bool bootstrappedEnabled; Not totally sure I like this name, and as you mentioned on IRC it should all be lower case. Maybe allowbootstrap might be better? @@ +118,2 @@ > NULL, &nsChromeRegistry::ManifestStyle }, > + { "override", 2, true, true, false, false, Don't think there is a reason not to allow override, include it and add a test for it. @@ +120,2 @@ > NULL, &nsChromeRegistry::ManifestOverride }, > + { "resource", 2, true, true, true, false, checkForNewChrome won't rebuild the resource protocol mappings so I don't think we can allow this just yet. Resource mappings can be registered and unregistered pretty easily from JS anyway so I don't think it is a big deal. @@ +451,5 @@ > > rv = xapp->GetVersion(s); > if (NS_SUCCEEDED(rv)) > CopyUTF8toUTF16(s, appVersion); > + Looks like only a whitespace change here? Just leave it. @@ +554,5 @@ > + token); > + continue; > + } > + > + if (directive->componentonly && NS_COMPONENT_LOCATION != aType && NS_BOOTSTRAPPED_LOCATION != aType) { I think you actually just want to test if the location is NS_SKIN_LOCATION, that matches what the log message is complaining about better. ::: xpcom/components/nsComponentManager.cpp @@ +2038,5 @@ > +NS_IMETHODIMP > +nsComponentManagerImpl::XRE_AddBootstrappedManifestLocation(nsILocalFile* aLocation) > +{ > + nsString path; > + aLocation->GetPath(path); This will return an nsresult, check that it is a success using NS_ENSURE_SUCCESS @@ +2059,5 @@ > + > + PRBool isJar = PR_FALSE; > + nsCOMPtr<nsILocalFile> manifest; > + nsString path; > + aLocation->GetPath(path); Same as above here @@ +2078,5 @@ > + //remove reference > + nsComponentManagerImpl::sModuleLocations->RemoveElement(elem, ComponentLocationComparator()); > + > + rv = cr->CheckForNewChrome(); > + if (NS_FAILED(rv)) return rv; You aren't returning anything sometimes here, just return rv. ::: xpcom/components/nsComponentManager.h @@ +167,5 @@ > + public: > + PRBool Equals(const ComponentLocation& a, const ComponentLocation& b) const { > + PRBool res; > + if (a.type == b.type && a.jar == b.jar) { > + a.location->Equals(b.location, &res); Check the nsresult of this too @@ +173,5 @@ > + } else { > + return PR_FALSE; > + } > + } > + }; I guess it might be better to add an == operator to ComponentLocation then you could just use the default comparator instead. I'll leave that to bsmedberg though ::: xpcom/components/nsIComponentManager.idl @@ +103,5 @@ > + > + /** > + * XRE_AddBootstrappedManifestLocation > + * > + * Adds a manifest location on runtime. Only available for bootstrapped addons This isn't really only available for bootstrapped add-ons, it's available to all but only adds a bootstrapped manifest location. @@ +107,5 @@ > + * Adds a manifest location on runtime. Only available for bootstrapped addons > + * > + * @param aLocation : A file with the manifest to be loaded > + */ > + void XRE_AddBootstrappedManifestLocation(in nsILocalFile aLocation); This naming doesn't quite fit with normal xpcom methods. How about just addBootstrappedManifestLocation and removeBootstrappedManifestLocation
Attachment #544644 -
Flags: review?(dtownsend) → review-
Assignee | ||
Comment 18•13 years ago
|
||
(In reply to comment #17) > Comment on attachment 544644 [details] [diff] [review] [review] > Proposed Patch > > Review of attachment 544644 [details] [diff] [review] [review]: > ----------------------------------------------------------------- > ::: chrome/test/unit/data/test_bug564667/loaded.manifest > @@ +1,2 @@ > > +content test2 test/ > > +locale test2 en-US test/ > > What is this file for? > That is to test that `manifest` directives work OK. > @@ +110,5 @@ > > +{ > > + //Add a manifest file > > + Components.manager.XRE_AddBootstrappedManifestLocation(MANIFEST_PATH); > > + > > + // Test Adding Content URL > > Indentation here is a little messed up > Nope, is on purpose. I did it to indicate which tests that depend on adding the manifest and which depend on removing the manifest. > @@ +173,5 @@ > > + } else { > > + return PR_FALSE; > > + } > > + } > > + }; > > I guess it might be better to add an == operator to ComponentLocation then > you could just use the default comparator instead. I'll leave that to > bsmedberg though > I tried that, but I found it was easier to add a comparator function. Anyway I'm open to changes. > @@ +107,5 @@ > > + * Adds a manifest location on runtime. Only available for bootstrapped addons > > + * > > + * @param aLocation : A file with the manifest to be loaded > > + */ > > + void XRE_AddBootstrappedManifestLocation(in nsILocalFile aLocation); > > This naming doesn't quite fit with normal xpcom methods. How about just > addBootstrappedManifestLocation and removeBootstrappedManifestLocation I just went with the same naming as XRE_AddManifestLocation, as I couldn't find a better name. If you say it fits better with addBootstrappedManifestLocation, I'll change it. For the rest of the comments I'll upload an updated patch when I have them ready.
Assignee | ||
Comment 19•13 years ago
|
||
Addressed comments by Mossop's review. I also had to make some changes on RegisterJarManifest because it had the location type hard-coded in the function body and jarred xpi tests were failing.
Assignee: nobody → colmeiro
Attachment #544644 -
Attachment is obsolete: true
Attachment #545587 -
Flags: review?(dtownsend)
Comment 20•13 years ago
|
||
Comment on attachment 545587 [details] [diff] [review] Patch with addressed comments Review of attachment 545587 [details] [diff] [review]: ----------------------------------------------------------------- This looks great. A couple of minor things to fix and then we can get bsmedberg to do the final sr. ::: chrome/test/unit/test_bug564667.js @@ +103,5 @@ > + > + //Add a manifest file > + Components.manager.addBootstrappedManifestLocation(manifestPath); > + > + // Test Adding Content URL I still dislike this indentation, it makes it look like some control structure is missing. If you want to separate these parts out how about wrapping them in a function? Or put separators in the form of comments like: // --------------- Chrome registered ---------------------- @@ +149,5 @@ > + // Test an unpackaged addon > + testManifest(UNPACKAGED_ADDON, gIOS.newFileURI(UNPACKAGED_ADDON).spec); > + > + // Test a packaged addon > + testManifest(PACKAGED_ADDON, "jar:" + gIOS.newFileURI(PACKAGED_ADDON).spec+"!/"); Put spaces around the + ::: xpcom/components/ManifestParser.cpp @@ +112,2 @@ > NULL, &nsChromeRegistry::ManifestLocale }, > + { "skin", 3, false, false, true, false, You seem to have changed the ischrome flag for this @@ +114,2 @@ > NULL, &nsChromeRegistry::ManifestSkin }, > + { "overlay", 2, true, false, false, false, This too
Attachment #545587 -
Flags: review?(dtownsend) → review+
Assignee | ||
Comment 21•13 years ago
|
||
Last comments addressed
Attachment #545587 -
Attachment is obsolete: true
Attachment #545739 -
Flags: review?(dtownsend)
Comment 22•13 years ago
|
||
Comment on attachment 545739 [details] [diff] [review] Patch with final fixes Looks good, over to bs for an sr pass
Attachment #545739 -
Flags: superreview?(benjamin)
Attachment #545739 -
Flags: review?(dtownsend)
Attachment #545739 -
Flags: review+
Comment 23•13 years ago
|
||
Comment on attachment 545739 [details] [diff] [review] Patch with final fixes >+NS_IMETHODIMP >+nsComponentManagerImpl::AddBootstrappedManifestLocation(nsILocalFile* aLocation) >+{ >+ nsString path; >+ nsresult rv = aLocation->GetPath(path); >+ NS_ENSURE_SUCCESS(rv,rv); I don't allow NS_ENSURE_* in any new code: please use if (NS_FAILED(rv)) return rv; >+NS_IMETHODIMP >+nsComponentManagerImpl::RemoveBootstrappedManifestLocation(nsILocalFile* aLocation) >+{ >+ nsresult rv; >+ nsCOMPtr<nsIChromeRegistry> cr = do_GetService("@mozilla.org/chrome/chrome-registry;1", &rv); Use mozilla::Services for this. >+ NS_ENSURE_SUCCESS(rv,rv); ditto re: NS_ENSURE_* in this function >diff --git a/xpcom/components/nsComponentManager.h b/xpcom/components/nsComponentManager.h >+ class ComponentLocationComparator >+ { >+ public: >+ PRBool Equals(const ComponentLocation& a, const ComponentLocation& b) const { >+ PRBool res; >+ if (a.type == b.type && a.jar == b.jar) { >+ nsresult rv = a.location->Equals(b.location, &res); >+ NS_ENSURE_SUCCESS(rv,rv); ditto, but assert here: equals should never fail >--- a/xpcom/components/nsIComponentManager.idl >+ >+ /** >+ * addBootstrappedManifestLocation >+ * >+ * Adds a bootstrapped manifest location on runtime. >+ * >+ * @param aLocation : A file with the manifest to be loaded >+ */ >+ void addBootstrappedManifestLocation(in nsILocalFile aLocation); This needs more documentation regarding the magic ".xpi" behavior. sr=me with those fixed
Attachment #545739 -
Flags: superreview?(benjamin) → superreview+
Assignee | ||
Comment 24•13 years ago
|
||
Attachment #545739 -
Attachment is obsolete: true
Attachment #547305 -
Flags: superreview?(benjamin)
Assignee | ||
Updated•13 years ago
|
Attachment #506249 -
Attachment is obsolete: true
Comment 25•13 years ago
|
||
Comment on attachment 547305 [details] [diff] [review] Patch with sr fixes addressed You missed comment 23 point 4. (In comment #23) > >+ class ComponentLocationComparator > >+ { > >+ public: > >+ PRBool Equals(const ComponentLocation& a, const ComponentLocation& b) const { > >+ PRBool res; > >+ if (a.type == b.type && a.jar == b.jar) { > >+ nsresult rv = a.location->Equals(b.location, &res); > >+ NS_ENSURE_SUCCESS(rv,rv); > > ditto, but assert here: equals should never fail
Assignee | ||
Comment 26•13 years ago
|
||
(In reply to comment #25) > Comment on attachment 547305 [details] [diff] [review] [review] > Patch with sr fixes addressed > > You missed comment 23 point 4. > > (In comment #23) > > >+ class ComponentLocationComparator > > >+ { > > >+ public: > > >+ PRBool Equals(const ComponentLocation& a, const ComponentLocation& b) const { > > >+ PRBool res; > > >+ if (a.type == b.type && a.jar == b.jar) { > > >+ nsresult rv = a.location->Equals(b.location, &res); > > >+ NS_ENSURE_SUCCESS(rv,rv); > > > > ditto, but assert here: equals should never fail Thanks for pointing that out, I was sure I had changed that, it might got lost when generating the patch or something. I'm adding a new patch now.
Assignee | ||
Updated•13 years ago
|
Attachment #547305 -
Flags: superreview?(benjamin)
Assignee | ||
Comment 27•13 years ago
|
||
Now it has all the nits addressed :)
Attachment #547305 -
Attachment is obsolete: true
Assignee | ||
Updated•13 years ago
|
Attachment #547334 -
Flags: superreview?(benjamin)
Comment 28•13 years ago
|
||
Comment on attachment 547334 [details] [diff] [review] Patch with sr fixes addressed The comments are addressed so this has sr.
Attachment #547334 -
Flags: superreview?(benjamin) → superreview+
Comment 29•13 years ago
|
||
Pushed to inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/ed32cfcfd3f0
Whiteboard: [rewrite] → [rewrite][inbound]
Comment 30•13 years ago
|
||
This patch broke the build, so I backed it out
Whiteboard: [rewrite][inbound] → [rewrite]
Comment 31•13 years ago
|
||
the problem was PRBool isJar, the third member of the struct was a bool, not a prbool, gcc doesn't like you to assign a PRBool to a bool. This passed try, so I pushed it. http://hg.mozilla.org/integration/mozilla-inbound/rev/acd21e50bd12
Whiteboard: [rewrite] → [rewrite][inbound]
Comment 32•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/acd21e50bd12
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [rewrite][inbound] → [rewrite]
Target Milestone: --- → mozilla8
Comment 33•13 years ago
|
||
Need to file followup bugs for auto-parsing chrome.manifest and for allowing resource:// mappings in chrome.manifest?
Comment 34•13 years ago
|
||
(In reply to comment #33) > Need to file followup bugs for auto-parsing chrome.manifest and for allowing > resource:// mappings in chrome.manifest? Please do!
Updated•13 years ago
|
Keywords: dev-doc-needed
Comment 35•13 years ago
|
||
Followup bugs filed: auto-load/unload -> bug 675371 allow resource:// -> bug 675372
Comment 36•13 years ago
|
||
I also filed bug 675387 for overlay directives, but there's no capability to unload them at this time so that one will probably have to wait a while. Do we care about supporting style overlay directives here, and if so is the capability also missing along the lines of XUL overlays?
Comment 37•13 years ago
|
||
(In reply to comment #36) > I also filed bug 675387 for overlay directives, but there's no capability to > unload them at this time so that one will probably have to wait a while. > > Do we care about supporting style overlay directives here, and if so is the > capability also missing along the lines of XUL overlays? I don't think there is an API to do it right now, but I think it'd be simpler to add one than for overlays for example. I believe style lines just wind up as xml-stylesheet lines in the DOM.
Comment 38•13 years ago
|
||
Hi guys. How can I verify this as being resolved fixed? Thanks
Comment 39•13 years ago
|
||
Documentation updated: https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIComponentManager https://developer.mozilla.org/en/Extensions/Bootstrapped_extensions#Notes_on_modifying_the_application_user_interface And mentioned on Firefox 8 for developers.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•