Closed Bug 73127 Opened 24 years ago Closed 24 years ago

Change how JS Console is posed

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla0.9.1

People

(Reporter: trudelle, Assigned: mikepinkerton)

References

()

Details

(Keywords: embed, Whiteboard: needs review)

Attachments

(2 files)

Need to change how JS Console is invoked so it can be overridden by embedding clients. Some details below, more at URL above; danm will be happy to help and answer questions. UI Posed by Gecko (Up Calls) ============================ These are calls to window.open, in one form or another, which are relevant to embedding. They're split into calls and callers, where a call is considered to be a base-level call to window.open which should be API-ized like we're discussing, and a caller is something which currently uses window.open but should use one of these new APIs when they're created. I've tried to categorize these sensibly... Each category implies a UI-posing component with methods for each item marked "CALL" in that category. Items marked "CALLER" in a given category may imply a new method in that component (eg. the print dialog stuff), or may just be a caller of some other extant method in a different component (eg. the uri loader opening a new window). JS Console: CALL: /dom/src/jsurl/nsJSProtocolHandler.cpp#228 in ::BringUpConsole, open the JS console window CALLER: /xpfe/components/console/resources/content/consoleBindings.xml#433 click handler in binding for message opens chrome://navigator/content/viewSource.xul
Adding mozilla0.9 keyword, dependency. cc brendan, who appears to have written this code.
Blocks: 71837
Keywords: mozilla0.9
Trying to find the right owner for this; cc'ing jband
Joe or I could probably do this.
blake, you wanna take this? It was a ben/mccabe special. Cc'ing ben too. It should not be assigned to rogerl or to me. /be
Okay. Mine, all mine!
Assignee: rogerl → blakeross
Status: NEW → ASSIGNED
Keywords: embed
Target Milestone: --- → mozilla0.9
No longer blocks: 71837
Blocks: 65233
Apologies for all the bug spam. For the detailed overview of this task, see danm's document: http://www.mozilla.org/xpfe/embedding-dialogs.html See my first cut at a component-wise categorization: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=27972 And if you want to laugh, see also my brainless incomplete IDL for these components: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=28175 dr
Will this make the 0.9 cutoff tonight? Please move the milestone if not.
Target Milestone: mozilla0.9 → mozilla0.9.2
Whoa! We have an embedding customer that needed this in 0.9. Landing very early in 9.1 would be tolerable, but 9.2 is unacceptable. Can we retarget, or do we need to reassign?
Sorry, the "enhancement" severity threw me off... I'll work on getting this into 9.1. Reassign if you're worried, but I'm not sure who'd be a better owner here.
Severity: enhancement → normal
Target Milestone: mozilla0.9.2 → mozilla0.9.1
Dan, from what I gather from reading the docs provided, the js console needs to become its own component with an interface so embedding customers can use it, right? That view source call happens when a user tries to access the filename that the error or warning lives in; assuming that needs to be fixed as well, suggestions on the most efficient way of doing that? I'll continue to work on this bug, but I missed the urgency of it when I signed up for it. I don't think I can commit to this within the next week, so this is probably best reassigned. Sorry.
Keywords: mozilla0.9mozilla0.9.1
changing keyword back to mozilla0.9. Anyone have an idea who else could take this work on in the next week or so?
Keywords: mozilla0.9.1mozilla0.9
Blake: We're currently reviewing whether the JS Console is part of the embedding API. I think it must be, but it's worth a second look. If so, then yes, you got it. The point of this exercise is to give an embedding app hooks to define and run its own secondary windows, so it can maintain a consistent look and feel. That means that any component or code that wants to open and run a new window/dialog needs to have all the code necessary for interacting with the window moved into an interface; the implementation for that interface needs to be instantiated through a factory registered in such a way that the embedding app can override it, and of course all the code that accesses the window must go through the API. Examples are creeping in. It's a lot of work. Bill is doing the same thing with the helper app dialog, for instance, in bug 52454. Everybody is swamped; I don't know who else wants this work if you don't. We'll have to think about finding someone with emergency cycles to take this on. If you end up keeping it -- if you want to and we have the time -- we can talk about implementation details offline. But Bill's bug is a good place to start looking.
Keywords: mozilla0.9mozilla0.9.1
Jud says the JS Console doesn't need to be part of the embedding UI. Upon looking a little closer, the console can't be opened accidentally from content; the Security Manager prevents that. It can be opened only by explicit request from application chrome. I still think this is something embedded apps could reasonably be interested in controlling. But even if so, it falls into that same category as the Form Fill Manager: to be added later. I'm confirming this bug's 0.9.1-hood (though it could reasonably be pushed to 1.0, I think) and cc:ing rginda, who may have opinions.
No longer blocks: 65233
Blocks: 65233
It turns out that the JS Console's refusal to appear upon an attempt to open the URL "javascript:" is bug 59748. With that fixed, Gecko will have the internal ability to open a XUL javascript window, and that, unless I'm mistaken, makes this bug an 0.9 embedding API issue once again.
Target Milestone: mozilla0.9.1 → mozilla0.9
Well then, it sounds like we need a new owner. Ben? Joe?
accepting my punishment.
Assignee: blakeross → pinkerton
Status: ASSIGNED → NEW
notes from discussion with rginda: - bug 59749 has a one-line patch to the c which needs to be made before this will work - code in nsJSProtocolHandler needs to be moved into its own component - interface can just have one method.
Status: NEW → ASSIGNED
oops, i meant 59748, but i'm sure you figured that out by now ;)
this goes to 0.9.1, but should be pretty easy.
Target Milestone: mozilla0.9 → mozilla0.9.1
done, just gotta get it reviewed
Whiteboard: needs review
Attached patch [patch] da patch (deleted) — Splinter Review
Well (!) I'd like to change a few things... Sorry... For starters, I'm still a little unhappy with this service being in the embedding components DLL. A little. As we've discussed, it'd be even worse to make a whole new DLL just for this tiny thing. But I would like it to be in a separate source directory; not part of the windowwatcher directory. Something like mozilla/ embedding/components/jsconsoleservice, maybe. Last, as long as we're touching the code, might as well modernise it. The technique of pushing JS Args to open a window is something the world never wants to see again. The entire nsJSConsoleService::Open method in the new world would be: NS_IMETHODIMP nsJSConsoleService::Open(nsIDOMWindow *aParent) { nsCOMPtr<nsIWindowWatcher> wwatch(do_GetService("@mozilla.org/embedcomp/window- watcher;1")); if (!wwatch) return NS_ERROR_FAILURE; return wwatch->OpenWindow(aParent, console_chrome_url, "_blank", console_window_options); } (and you're not losing anything by invoking yet another service, since that's what's behind the original window->OpenDialog, anyway). And of course NS_WITH_SERVICE(nsIJSConsoleService, jsconsole, "@mozilla...") is happier as nsCOMPtr<nsIJSConsoleService> jsconsole(do_GetService("@mozilla...")) that stuff, and r=me.
Priority: -- → P1
landed.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Marking Verified -
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: