I had blindly assumed that would work. And if this is the case for all chrome:// URLs loaded in a tab, then its going to be quite an issue as more things get moved from being windows/dialogs to being loaded in tabs (eg, preferences, download manager, places library, etc). Neil: Any ideas? Or ideas on anyone else I should ask? I'm really hoping this is something that can be fixed.
A test on a simple overlay works fine. So may be something specific to the extensions. Is extensions.xul loaded in the normal way? That it applies when calling open, as described in the first comment, implies that the manner in which the file is loaded and displayed in involved.
Ah, is the extension window actually being loaded as about:extensions or somesuch? An overlay on chrome://mozapps/content/extensions/extensions.xul won't work. Overlays also only work for chrome urls.
"is the extension window actually being loaded as about:extensions or somesuch?" Yes about:addons
Yes, its normally loaded as about:addons. But Brian said to me that he also tried loading the chrome:// URL directly with no success. Brian - can you confirm? Would there be any possibility of getting overlays working for about: URLs? Or is there something the about: protocol handler can do to make them work? AFAIK, the plan is to introduce more chrome pages loaded in tabs as about: URLs. Not being able to load overlays on those pages would be a deal-breaker; and we'd have to consider using chrome:// URLs instead, which really isn't wanted.
(In reply to comment #5) > Yes, its normally loaded as about:addons. But Brian said to me that he also > tried loading the chrome:// URL directly with no success. Brian - can you > confirm? Sorry, I was mistaken. I just tested via chrome://mozapps/content/extensions/extensions.xul and the overlay *is* loaded. > Would there be any possibility of getting overlays working for about: URLs? Or > is there something the about: protocol handler can do to make them work? > > AFAIK, the plan is to introduce more chrome pages loaded in tabs as about: > URLs. Not being able to load overlays on those pages would be a deal-breaker; > and we'd have to consider using chrome:// URLs instead, which really isn't > wanted. Yes, there are many add-ons out there that overlay the Add-ons Manager that will be broken by this.
Just weighing in to say that Greasemonkey is very interested in listing User Scripts in the standard "Add Ons" dialog, wherever it may be, as we view User Scripts as just another kind of add on for the browser, and believe it's simplest/best to present them that way to users. We would desperately appreciate if the Firefox developers could reach out to us as this change progresses, to make sure that this is both possible and straightforward (i.e. without fragile techniques). Our developer list is greasemonkey-dev @googlegroups.com . Thanks in advance.
The last time we came across this we closed it as basically intentional. I wonder how hard it would be to implement though.
Allowing overlays on about: urls turns out to be very easy, at least on the surface. What I don't know is if there was a specific reason it is disabled right now. Do you know Boris?
Where is it disabled?
Created attachment 445373 [details] [diff] [review] patch rev 1 Perhaps easiest answered with this WIP patch I have that enables overlays for about: uris. I still need to verify what that means for unprivileged about: uris.
OK, why not ask Benjamin, who added those checks? They were added in bug 229285.
IIRC it had something to do with fastload of overlays and that the fastload cache was correct if you loaded a page via chrome: first and then via about:, but not the other way around. I don't have a strong preference as long as it works consistently.
So we have a patch here, and what seems like agreement that we can do this. Can we check in?
We have agreement? What we have is a note that doing this in the past broke things and that if we want to do it now we need to make sure said things aren't broken first. Ideally in automated test form, of course. See comment 13.
Yeah I need to work out how to verify and if possible test the problem mentioned in comment 13.
Comment on attachment 445373 [details] [diff] [review] patch rev 1 I think this patch is correct, at least as far as my understanding goes here. From bsmedbergs concerns I have verified that it works regardless of whether you load the about: page or chrome: page first. I couldn't figure out exactly whether the documents were going into the XUL cache correctly, I'm not sure how to do that. I also don't know how we can automatically test this easily since we'd have to tell the chrome registry about new overlays somehow. I can easily provide an add-on that overlays about:addons to prove it works in a litmus test though if we have nothing else. This should cover overlay and style overlays.
> From bsmedbergs concerns I have verified that it works regardless of whether > you load the about: page or chrome: page first. Hmm. So what I seem to recall is that scripts in overlays are compiled with the principal of the document being overlayed and then cached in the xul proto cache. Which means that if you overlay an about: page which is not system-principal, it will "break" the scripts in that overlay if it's applied to any system-principal document, right? And due to fastload, that will persist across restarts, even.... I guess the IsChromeURI guard in nsXULDocument::OnStreamComplete is staying, though, right? Should we add conditions in nsXULDocument::LoadOverlayInternal to not stick overlays being loaded into the proto cache if |this| is not chrome? It would be slower for about: uses which use overlays, but safer, I think.
Created attachment 448362 [details] ACR dev XPI Dave, in comment 17 you were looking for an add-on to test. This attachment is a dev version of the Add-ons Compatibility Reporter. There is only a content overlay, not a style one: overlay chrome://mozapps/content/extensions/extensions.xul chrome://acr/content/view/extensionsOverlay.xul What you should see is a Compatibility button in the listings in the add-ons window.
Created attachment 449284 [details] [diff] [review] patch rev 1 (In reply to comment #18) > Should we add conditions in nsXULDocument::LoadOverlayInternal to not stick > overlays being loaded into the proto cache if |this| is not chrome? It would > be slower for about: uses which use overlays, but safer, I think. Added that. I've also made it so we ignore overlays in the cache when the main document is not chrome to avoid the case where the main document does not have the system principal but the cached overlay would, which I presume would also be bad. It might be nice to make it so we really check if the main document has the system principal or not rather than just if it is chrome at some point but I think this works for now. I've verified with some logging that we don't read/write the cache when loading about: urls with overlays but still do for chrome:.
Per the planning meeting we want the add-on compatibility reporter available and working in time for beta1 and this bug blocks that so it is a beta blocker.
NEW ACR test version compatible with 3.7a6pre: https://bugzilla.mozilla.org/attachment.cgi?id=450640
So does that new version work with the trunk? Or is it still blocked on this bug? bz: any chance of a review early next week?
(In reply to comment #23) > So does that new version work with the trunk? Or is it still blocked on this > bug? ACR is still blocked by this.
> bz: any chance of a review early next week? Yes. Maybe even tonight, if I really need a break from this e10s merge.
Comment on attachment 449284 [details] [diff] [review] patch rev 1 >+++ b/content/xul/document/src/nsXULDocument.cpp >+ // why is this check a member function of nsXULDocument? -gagan It's not, and gagan's not involved. Just drop that line, please. >+ // the cached document will do, see bug 565610. s/will do/will/ r=bzbarsky
Thanks, landed: http://hg.mozilla.org/mozilla-central/rev/5f3f36fe8d76
Confirmed WORKING with ACR with following entry in chrome.manifest: overlay about:addons chrome://acr/content/view/extensionsOverlay.xul r69115
Tested with latest developer release of ACR and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a6pre) Gecko/20100624 Minefield/3.7a6pre on all platforms. The Add-ons Manager can be overlayed now. Marking as verified fixed.