Closed
Bug 119923
Opened 23 years ago
Closed 19 years ago
[minimo]implement minimal chrome registry
Categories
(Minimo Graveyard :: General, defect, P1)
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: waterson, Assigned: alecf)
References
Details
(Keywords: embed, topembed-, Whiteboard: chimera)
Attachments
(6 files, 2 obsolete files)
54.67 KB,
patch
|
Details | Diff | Splinter Review | |
5.66 KB,
text/plain
|
Details | |
13.45 KB,
patch
|
Details | Diff | Splinter Review | |
1012 bytes,
patch
|
Details | Diff | Splinter Review | |
2.66 KB,
text/plain
|
Details | |
9.28 KB,
patch
|
Details | Diff | Splinter Review |
We could probably break the dependency of embedding on RDF (and perhaps a few other modules) if we implemented a `minimal' chrome registry. Such an implementation might be a hard-coded list of lookups for scrollbars, etc.
Assignee | ||
Comment 1•23 years ago
|
||
I hope I can get to this for moz 1.0 :)
Target Milestone: --- → mozilla1.0
Comment 2•23 years ago
|
||
This is an issue for Chimera. Hopefully no one minds if I adorn the status whiteboard to indicate that. Let me know if you need any help/support with this. IMO you just have to make an in-memory table mapping for the packages, skins, and locales listed in installed-chrome.txt in embedding/config. We should probably also do something like patch NS_InitEmbedding (which presumably isn't called by the main app?) to register the simpler chrome registry automatically. Another note. Make sure not to eliminate the user stylesheet support. That should carry over to a simpler chrome registry.
Whiteboard: chimera
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P2
Comment 3•22 years ago
|
||
This is not related to a top priority embedding project at this point. Minusing to topembed- and adding embed.
Assignee | ||
Comment 4•22 years ago
|
||
nsIChromeRegistry is huge. I might cut it up to make implementation simpler (and just force some existing consumers to QI())
Assignee | ||
Comment 5•22 years ago
|
||
ok, looking for an sr=hyatt here :) I've broken out some of the routines from nsIChromeRegistry into 3 interfaces: - nsIChromeInstaller - contains all the runtime-install stuff that xpinstall uses, such as installSkin(), etc - nsIChromePackageRegistry - contains all the per-package stuff, like selectSkinForPackage(), isLocaleSelectedForPackage(), etc - nsIChromeRegistry - everything else While I was there, I also did some big cleanups to switch lots of internal methods from nsCString& to nsACString&, which allowed us to remove a ton of nsCAutoStrings, and use NS_LITERAL_CSTRING instead (which should be faster) None of this has any real effect on the existing chrome registry implementation. I'd like to land this so that the embedding-only registry can simply implement nsIChromeRegistry - that seems to be all we need to have an embedding registry. reviews?
Comment 6•22 years ago
|
||
I don't really see the necessity of splitting up nsIChromeInstaller and nsIChromePackageRegistry. Both deal with the installation and selection of skins, and IMO conceptually they belong together. "Package" is also the incorrect term to use here, since the interface contains skin and locale methods. I'd prefer to just see two interfaces: (1) nsIXULChromeRegistry (which contains all the methods of your nsIChromeInstaller and nsIChromePackageRegistry) (2) nsIChromeRegistry (the base interface) nsIXULChromeRegistry should *inherit* from nsIChromeRegistry so that JS code that wishes to use the base chrome registry methods doesn't have to QI. Be aware that there is JS code that uses nsIChromeRegistry, so you'll need to patch it to use nsIXULChromeRegistry instead. (The code that selects skins dynamically probably does this. Although it may be commented out, it should still be patched.)
Comment 7•22 years ago
|
||
Also, the code for agent sheets and user sheets needs to be in the minimal chrome registry. That is not part of the XUL chrome registry. Ultimately this code should be broken out into a completely different object (that isn't a chrome registry at all), but that can wait. The point is that it is used by embedding and is part of a minimal registry.
Assignee | ||
Comment 8•22 years ago
|
||
Ok, thanks for the suggestions, I'll reduce this to 2 interfaces. do you have any other suggestions for what goes in nsIXULChromeRegistry? I had a few questionable routines: * boolean allowScriptsForSkin(in nsIURI url); is scriptablity something XUL-specific? * void reloadChrome(); if chrome isn't XUL does this method make any sense? Maybe it does in case chimera or embedding wants a call to reload chrome? * void getAgentSheets(in nsIDocShell docShell, out nsISupportsArray styleSheets); the implementation is very XUL-specific * nsISimpleEnumerator getOverlays(in nsIURI aChromeURL); overlays also seem to be xul-specific. In fact, I'm wondering if any of the "skin" or "locale" related stuff even makes sense in nsIChromeRegistry... if that's the case, it looks like all the basic chrome registry stuff does is deal with chrome URLs. Thoughts?
Comment 9•22 years ago
|
||
getAgentSheets *is* relevant for embedding and would be part of a minimal chrome registry, since you need to (at the very least) supply the scrollbar sheet for embedding. User sheets should be part of the base registry as well. Everything dealing with skins and locales should just be part of nsIXULChromeRegistry IMHO. Of the methods you listed, all belong in nsIXULChromeRegistry except for getAgentSheets.
Comment 10•22 years ago
|
||
You should sanity-check all the locale stuff. They may have code that asks for the current locale in embedding. I'm not sure.
Assignee | ||
Comment 11•22 years ago
|
||
ok, I've taken hyatt's suggestions and implemented them... nsIChromeRegistry is now split into nsIChromeRegistry and nsIXULChromeRegistry, and I've updated all the C++ and JS I could find (via lxr) that would affect this. hyatt, mind taking a look, and possibly sr'ing?
Attachment #77464 -
Attachment is obsolete: true
Assignee | ||
Comment 12•22 years ago
|
||
the resulting .idl file, to make things clearer
Comment 13•22 years ago
|
||
Yeah, this split looks reasonable.
Comment 14•22 years ago
|
||
Comment on attachment 77717 [details] [diff] [review] Split nsIChromeRegistry into 2 interfaces sr=hyatt
Attachment #77717 -
Flags: superreview+
Reporter | ||
Comment 15•22 years ago
|
||
Lose this, and r=waterson. @@ -1188,6 +1187,10 @@ static nsresult main1(int argc, char* argv[], nsISupports *nativeApp ) { nsresult rv; + + printf("sizeof(nsStr) = %d\n", sizeof(nsStr)); + printf("sizeof(nsString) = %d\n", sizeof(nsString)); +
Assignee | ||
Comment 16•22 years ago
|
||
Comment on attachment 77717 [details] [diff] [review] Split nsIChromeRegistry into 2 interfaces oops! printf's removed. I'm adding the has-review flag to make it easier for drivers to see that the patch is ready (also grepped the rest of the patch for printfs, just to be sure!)
Attachment #77717 -
Flags: review+
Updated•22 years ago
|
Attachment #77717 -
Flags: approval+
Assignee | ||
Comment 17•22 years ago
|
||
ok, with a little trouble, this patch has landed on the trunk. tomorrow's builds will have this, so hopefully all will be well. I tested everything I could think of...
Comment 18•22 years ago
|
||
Adding adt1.0.0-. We can ship Mach V beta without this and this is topembed-. If this is needed for Mozilla1.0, please land after Mach V branches.
Assignee | ||
Updated•22 years ago
|
Target Milestone: mozilla1.0 → mozilla1.1beta
Comment 19•22 years ago
|
||
mark fixed ?
Assignee | ||
Comment 20•22 years ago
|
||
I'll do that when I actually fix the bug.
Assignee | ||
Comment 21•22 years ago
|
||
*** Bug 68832 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 22•22 years ago
|
||
moving bugs to mozilla 1.2alpha that missed 1.1beta
Target Milestone: mozilla1.1beta → mozilla1.2alpha
Assignee | ||
Comment 23•22 years ago
|
||
This converts all consumers of the chrome registry over to using a ContractID, so that I can do a drop-in replacement by just registering under the contractid. It also makes the CID private, so it can only be referenced from the chrome DLL. I'll have a patch for the commercial tree momentarily. Can I get reviews?
Assignee | ||
Comment 24•22 years ago
|
||
here's the commercial patch.
Assignee | ||
Comment 25•22 years ago
|
||
Comment on attachment 90978 [details] [diff] [review] convert CID to ContractID in commercial tree ignore the fact that we're switching from nsIChromeRegistry to nsIXULChromeRegistry - the code is #if 0'ed, and we will need to use nsIXULChromeRegistry if this code is ever turned on.
Assignee | ||
Updated•22 years ago
|
Attachment #90977 -
Attachment description: contvert CID to ContractID in all consumers → convert CID to ContractID in all consumers
Comment 26•22 years ago
|
||
Comment on attachment 90977 [details] [diff] [review] convert CID to ContractID in all consumers please be consistent in your use of NS_CHROMEREGISTRY_CONTRACTID vs. "@mozilla.org/chrome/chrome-registry;1" in your do_GetService calls. with that... r=bnesse.
Attachment #90977 -
Flags: review+
Updated•22 years ago
|
Attachment #90978 -
Flags: review+
Comment 27•22 years ago
|
||
Comment on attachment 90978 [details] [diff] [review] convert CID to ContractID in commercial tree r=bnesse.
Comment 28•22 years ago
|
||
Comment on attachment 90978 [details] [diff] [review] convert CID to ContractID in commercial tree sr=dveditz
Attachment #90978 -
Flags: superreview+
Comment 29•22 years ago
|
||
Comment on attachment 90977 [details] [diff] [review] convert CID to ContractID in all consumers >Index: rdf/chrome/public/nsIChromeRegistry.idl >=================================================================== >+#define NS_CHROMEREGISTRY_CONTRACTID \ >+ "@mozilla.org/chrome/chrome-registry;1" > %} When I added the embeddable interfaces for XPInstall I got slapped during super-review for including my contractid define in the .idl file ("the contract id refers to a COMPONENT, the .idl describes an INTERFACE"). I had to include the contract ID as a comment instead. Bah. This way makes more sense to me than having all the consumers define it themselves (or include yet another header file just for that) and I prefer it, but thought I should mention the other line of thought. >+ do_GetService("@mozilla.org/chrome/chrome-registry;1", &rv); You're changing these to the #define, right? sr=dveditz
Attachment #90977 -
Flags: superreview+
Comment 30•22 years ago
|
||
Comment on attachment 90977 [details] [diff] [review] convert CID to ContractID in all consumers a=asa (on behalf of drivers) for checkin to 1.1
Attachment #90977 -
Flags: approval+
Assignee | ||
Updated•22 years ago
|
Priority: P2 → P1
Assignee | ||
Comment 31•22 years ago
|
||
moving some stuff out to mozilla1.2beta that didn't make the mozilla1.2alpha train
Target Milestone: mozilla1.2alpha → mozilla1.2beta
Assignee | ||
Comment 32•22 years ago
|
||
unfortunately RDF is too ingrained in the chrome registry right now to be able to fix this for 1.2. Moving out to 1.3, which will require a change to the chrome registry format, probably to some sort of "contents.txt" solution.
Target Milestone: mozilla1.2beta → mozilla1.3beta
Assignee | ||
Updated•22 years ago
|
Target Milestone: mozilla1.3beta → mozilla1.4beta
Updated•21 years ago
|
Summary: implement minimal chrome registry → [minimo]implement minimal chrome registry
Assignee | ||
Comment 33•21 years ago
|
||
ok, I've made some signifigant progress here. Here's how I'm going to tackle this: - regchrome is going to have a new option -s to generated a "simplified" chrome registry given an existing chrome.rdf file. This will basically read the current chrome.rdf and turn it into a chrome.properties which will be a sort of "flat file" version of the chrome registry - a simplified chrome registry implementation will read chrome.properties and will be able to answer all the same questions to get chrome URLs mapped to real URLs. - when you go to build the embedding client, the build system will generate embed.jar, and then sic regchrome on it to generate a chrome.properties for the file. When you distribute the embedding client, all you need to distribute is the chrome.properties file This means we'll still need RDF at BUILD time in order to process all the contents.rdf files that exist in embed.jar, but once they are processed, we won't need rdf.dll at runtime! The other nice thing this means is that the only changes are: - regchrome.cpp - to add this new capability (mostly complete) - build system - to support generation of this new file so no changes are necessary to the core gecko product.
Assignee | ||
Comment 34•21 years ago
|
||
ok, this patch adds a switch "-p" to regchrome which allows someone to specify a flat-file .properties file with all the data in the chrome registry. without the -p, the behavior is the same. I'll attach a sample embed-chrome.properties that I generated from the embedding build.
Assignee | ||
Comment 35•21 years ago
|
||
here is a .properties file that could be read by a bare-bones chrome registry.
Assignee | ||
Updated•21 years ago
|
Attachment #121349 -
Attachment is patch: false
Assignee | ||
Comment 36•21 years ago
|
||
Comment on attachment 121347 [details] [diff] [review] ability to output to a .properties file requesting reviews from dougt and bryner - this will allow us to have a lightweight chrome registry (which could be used by embedding and/or camino...)
Attachment #121347 -
Flags: superreview?(bryner)
Attachment #121347 -
Flags: review?(hyatt)
Comment 37•21 years ago
|
||
Comment on attachment 121347 [details] [diff] [review] ability to output to a .properties file >+ >+ // remove the prefix, the provider type, and the trailing ':' >+ // (the compiler will optimize out the -1 + 1 close the parentheses. Looks good. sr=bryner.
Attachment #121347 -
Flags: superreview?(bryner) → superreview+
Assignee | ||
Comment 38•21 years ago
|
||
ok, I was missing selectedSkin and selectedLocale because they followed resources, not literals. sorry I'll need reviews again :(
Attachment #121347 -
Attachment is obsolete: true
Assignee | ||
Comment 39•21 years ago
|
||
Comment on attachment 121528 [details] [diff] [review] ability to output to a .properties v1.1 hyatt/bryner - sorry I need a review on this again - some extra tweaks to handle selectedSkin/selectedLocale
Attachment #121528 -
Flags: superreview?(bryner)
Attachment #121528 -
Flags: review?(hyatt)
Assignee | ||
Updated•21 years ago
|
Attachment #121347 -
Flags: review?(hyatt)
Assignee | ||
Comment 40•21 years ago
|
||
I've also been hacking on nsEmbedChromeRegistry and am almost done implementing nsIChromeRegistry such that it can auto-resolve URLs. the only other trick may be looking at calls to nsIXULChromeRegistry::GetSelectedLocale() to see if they should be converted to something more generic.
Comment 41•21 years ago
|
||
Comment on attachment 121528 [details] [diff] [review] ability to output to a .properties v1.1 sr=me
Attachment #121528 -
Flags: superreview?(bryner) → superreview+
Comment 42•21 years ago
|
||
Comment on attachment 121528 [details] [diff] [review] ability to output to a .properties v1.1 r=hyatt
Attachment #121528 -
Flags: review?(hyatt) → review+
clearing target milestone, since 1.4 is long gone.
Target Milestone: mozilla1.4beta → ---
Assignee | ||
Comment 44•21 years ago
|
||
just for my own reference, all of the above patches have been checked in. next step, a chrome registry that can read .properties files.
Target Milestone: --- → mozilla1.7beta
Comment 45•21 years ago
|
||
moving minimo bugs to the new bugzilla product.
Component: XP Toolkit/Widgets: XUL → General
Product: Browser → Minimo
Target Milestone: mozilla1.7beta → ---
Version: Trunk → unspecified
Comment 46•19 years ago
|
||
ben, any interest in this direction? alec, any continued interest?
Comment 47•19 years ago
|
||
marking as wont fix. benjamin has done a great job on mozilla/chrome. that is what minimo will use.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•