Stan Shebs Reporter
	Description • 18 years ago

The old Carbon-based Apple Events code in xpfe/bootstrap/appleevents should not be built or linked into Cocoa configs; it is redundant with Cocoa delegate-based handling added in 355352.

There are a couple technical problems that will need to be solved:

1) In toolkit/xre/nsCommandLineServiceMac.cpp, ProcessAppleEvents() handles files passed by double-clicking on them. Not clear if removing the call will let the app delegate handle everything via odoc's.

2) The Carbon AE also purports to set up a bunch of scripting vocabulary that one might use to control browser from AppleScript. Not clear if any of this works, or is expected to work (my guess is no to both).

	Stan Shebs Reporter
	Comment 1 • 18 years ago

I can't help myself, I'm testing the whack-out now...

	Stan Shebs Reporter
	Comment 2 • 18 years ago

Josh, you might as well assign to me, I'm just about done with this one, only have startup sequence tinkering left to do.

	Josh Aas Assignee
	Comment 3 • 18 years ago

Any update on this?

	Stan Shebs Reporter
	Comment 4 • 18 years ago

Heh, well, that "startup sequence" is trickier than it seems. The AE bits are tied in with the fake command-line stuff, and the restart stuff, and so on, so without AE, there are several behavioral regressions, like double-clicking on a file no longer works. I've backburnered this one for now, if you want the current state of the patch I can attach it.

	Josh Aas Assignee
	Comment 5 • 16 years ago

This has the same behavioral regressions that Stan mentioned, but it compiles without all the carbon AppleScript code. Just wanted to leave it here as a place to start later on.

	Josh Aas Assignee
	Comment 6 • 15 years ago

Updated patch to remove carbon apple event handling. This probably has some functional regressions which we'll need to fix by adding Cocoa handlers for some apple events.

	Josh Aas Assignee
	Comment 7 • 15 years ago

updated to current mozilla-central

	Josh Aas Assignee
	Comment 8 • 15 years ago

some saved work, not useful for anything

	Josh Aas Assignee
	Comment 9 • 15 years ago

This removes the Carbon apple event code. Almost all of the functionality and more has been replaced by the default Cocoa script suites. The only thing we're losing is get/set URL in the front-most window, that scripting API was badly designed and doesn't belong in generic Gecko anyway. Script can still open URLs in Firefox other ways, by telling the app to open a simple URL file for example. We should file a bug on exposing a script suite for Firefox and whatever other apps wants suites specific to them.

The following example scripts work with this patch:

tell application "Firefox"
	return name of first window as text
end tell

tell application "Firefox"
	activate
	open "/Users/josh/Desktop/foo.url"
end tell

	Steven Michaud [:smichaud] (Retired)
	Comment 10 • 15 years ago

Comment on attachment 363400 [details] [diff] [review]
fix v1.0

This (basically) looks fine, and the tests you describe in comment #9
both work fine.

But it breaks the 'open' command.

When a build made with this patch is running, and Firefox is the
default browser, and you enter 'open filename.html' at a Terminal
prompt, the browser is made active, but nothing else happens.  What
should happen (and what does happen even with today's Minefield
nightly) is that the browser becomes active and 'filename.html' loads.

For that reason I'm going to r- this patch.

I suspect it's ProcessAppleEvents() in nsCommandLineServiceMac.cpp
that makes the 'open' command work properly ... but I didn't have time
to test this.

	Steven Michaud [:smichaud] (Retired)
	Comment 11 • 15 years ago

> But it breaks the 'open' command.

Oddly, it doesn't break double-clicking on local files, though.

	Steven Michaud [:smichaud] (Retired)
	Comment 12 • 15 years ago

Josh, I've figured out why the 'open' command doesn't work with	your
patch, and how to fix the problem:  The 'open' command sends a GURL
AppleEvent (http://www.scripting.com/midas/geturl.html).  So all you need
to do is install a handler for this event (using [NSAppleEventManager
setEventHandler:andSelector:forEventClass:andEventID:]).

This is what Safari does.  I found all this out using a little reverse
engineering :-)

For the record, Safari uses [NSAppleEventManager setEventHandler:...] to
install handlers for the following Apple events:

GURL/GURL
WWW!/OURL
aevt/oapp
aevt/rapp
aevt/odoc
aevt/pdoc
aevt/ocon
aevt/quit
aevt/actq
SECF/test

	Josh Aas Assignee
	Comment 13 • 15 years ago

Adds basic support for gurl events.

	Stuart Morgan
	Comment 14 • 15 years ago

Comment on attachment 364528 [details] [diff] [review]
fix v1.1

r=me in that the new code looks fine, but I'm not familiar with the old AE code at all (or what you're doing it terms of other Cocoa-level AE handling) so I have no idea what other regressions this could potentially introduce.

>+    NSString* schemeString = [[NSURL URLWithString:urlString] scheme];
>+    if (!schemeString ||
>+        [schemeString compare:@"chrome"
>+                      options:NSCaseInsensitiveSearch
>+                        range:NSMakeRange(0, [schemeString length])] == NSOrderedSame) {
>+      return;

Do you want javascript: URLs to work? If so, you won't want to use NSURL. It will choke on the unescaped JS that those URLs often contain and return nil, so you'll return early.

Have you tested handling of open when Firefox *isn't* already running with this change?

	Josh Aas Assignee
	Comment 15 • 15 years ago

Comment on attachment 364528 [details] [diff] [review]
fix v1.1

This does work when the app isn't already running and js urls don't matter. That AE handler is only there to cover the common case of using "open" on a local document, the fact that it works with standard URLs like http/https URLs is a bonus at this point. We'll be re-examining extended apple event support in the future.

	Josh Aas Assignee
	Comment 16 • 15 years ago

pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/3304d08b9dc3

	Smokey Ardisson (offline for a while; not following bugs - do not email)
	Comment 17 • 15 years ago

(In reply to comment #12)
> WWW!/OURL

As long as you're planning on matching Safari's AEs before this ships, you're good, but just FYI, you need to make sure you implement support for WWW!OURL (even in one of the "hidden" ways that Safari and Camino use) or you'll break Dreamweaver and a large number of other apps.  

That (bug 403346 et seq) was one of the two most difficult regressions to track down and fix after peeja rewrote Camino's AppleScript support (the other being bug 415510 et seq).

	Josh Aas Assignee
	Comment 18 • 15 years ago

Thanks for the info about DW. Someone should test and file a bug if something doesn't work.

	Smokey Ardisson (offline for a while; not following bugs - do not email)
	Comment 19 • 15 years ago

Moving this back to a live component where it can be found in searches, since it's a change made 3 months ago that will be new in 1.9.2, not some change from 2001 :P