Last Comment Bug 608049 - Add Applescript Support to Firefox
: Add Applescript Support to Firefox
Status: NEW
:
Product: Firefox
Classification: Client Software
Component: Shell Integration (show other bugs)
: unspecified
: x86 Mac OS X
-- normal with 26 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
: Robert Strong [:rstrong] (use needinfo to contact me)
Mentors:
: 237733 (view as bug list)
Depends on:
Blocks: 369901 490830 516502
  Show dependency treegraph
 
Reported: 2010-10-28 11:43 PDT by Scott Greenlay [:sgreenlay]
Modified: 2016-02-07 04:30 PST (History)
28 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Applescript WIP (v1.0) (61.50 KB, patch)
2010-11-03 16:40 PDT, Scott Greenlay [:sgreenlay]
no flags Details | Diff | Splinter Review
Applescript sample (3.31 KB, application/octet-stream)
2010-11-03 16:41 PDT, Scott Greenlay [:sgreenlay]
no flags Details
Applescript WIP (v1.1) (63.30 KB, patch)
2010-11-04 17:00 PDT, Scott Greenlay [:sgreenlay]
no flags Details | Diff | Splinter Review
Applescript support (v1.0) (65.59 KB, patch)
2010-11-08 16:07 PST, Scott Greenlay [:sgreenlay]
no flags Details | Diff | Splinter Review
Applescript support (v1.1) (68.22 KB, patch)
2010-11-09 15:38 PST, Scott Greenlay [:sgreenlay]
no flags Details | Diff | Splinter Review
Applescript support (v1.2) (68.19 KB, patch)
2010-11-12 14:31 PST, Scott Greenlay [:sgreenlay]
no flags Details | Diff | Splinter Review
Applescript support (v1.3) (68.38 KB, patch)
2010-12-14 14:59 PST, Scott Greenlay [:sgreenlay]
no flags Details | Diff | Splinter Review
Applescript sample (v1.1) (5.16 KB, application/octet-stream)
2010-12-14 15:02 PST, Scott Greenlay [:sgreenlay]
no flags Details
Applescript support (v1.4) (68.41 KB, patch)
2011-04-14 21:02 PDT, Scott Greenlay [:sgreenlay]
no flags Details | Diff | Splinter Review

Description User image Scott Greenlay [:sgreenlay] 2010-10-28 11:43:06 PDT
This is a tracking bug for adding basic applescript support to Firefox.
The design docs can be found at: https://wiki.mozilla.org/Mac:AppleScript
Comment 1 User image Scott Greenlay [:sgreenlay] 2010-11-03 16:40:06 PDT
Created attachment 488066 [details] [diff] [review]
Applescript WIP (v1.0)

Adds support for the following:

application
    windows          (read-only)
        name         (read-only)
        tabs         (read-only)
            index    (read-only)
            URL      (read-write)
            name     (read-only)
            source   (read-only)
            text     (read-only)
            selected (read-only)

Still working on opening new windows, new tabs and getting the currently selected tab/window.
Comment 2 User image Scott Greenlay [:sgreenlay] 2010-11-03 16:41:48 PDT
Created attachment 488067 [details]
Applescript sample

Current sample script that I have been running (gets all parameters for all windows and tabs, then sets all the tabs to go to google.com)
Comment 3 User image Scott Greenlay [:sgreenlay] 2010-11-04 17:00:27 PDT
Created attachment 488372 [details] [diff] [review]
Applescript WIP (v1.1)

Small update to fix collisions with IPC code.
Comment 4 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2010-11-06 18:25:19 PDT
I realize that that's a WIP patch, but I noticed a few minor things:

- Initial developer in your license headers should be "the Mozilla Foundation", not "Mozilla Corporation" (per http://weblogs.mozillazine.org/gerv/archives/2010/02/mpl_initial_developer_for_mozilla_employ.html )
- getService never returns null in JS, so no use null checking its result (it will throw on failure)
- you don't want to be setting window.name in getTabsForWindow, and the GeckoTab title getter should be using GetTitle() on the window's document.
Comment 5 User image Scott Greenlay [:sgreenlay] 2010-11-08 16:07:47 PST
Created attachment 489011 [details] [diff] [review]
Applescript support (v1.0)
Comment 6 User image Josh Matthews [:jdm] 2010-11-08 16:12:57 PST
There are a lot of blocks like

if (someCondition)
{
  ...
}
return

in the patch.  You could save a lot of indentation by converting those to early returns.  There's also some funky indentation going on in places (check out the bugzilla interdiff, that's where I noticed most of it).  Finally, you don't need to initialize or clear nsCOMPtr/nsRefPtr variables in the constructor and destructor - they're set to nsnull automatically.
Comment 7 User image Scott Greenlay [:sgreenlay] 2010-11-09 15:38:15 PST
Created attachment 489322 [details] [diff] [review]
Applescript support (v1.1)
Comment 8 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2010-11-10 13:35:14 PST
Comment on attachment 489322 [details] [diff] [review]
Applescript support (v1.1)

>diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js

>+     getWindows : function() {

Shouldn't this use isFullBrowserWindow too?

>+     getTabsInWindow : function(index) {
>+       var array = Cc["@mozilla.org/array;1"].
>+                   createInstance(Ci.nsIMutableArray);
>+       let win = this.getWindow(index);

         Array.forEach(gBrowser.browsers, function (b) { array.appendElement(b.contentWindow);});

(it shouldn't be possible for contentWindow to be null)

>+     getCurrentTabInWindow : function(index, tab_index) {
>+        let win = this.getWindow(index);
          return win.content; // (== win.gBrowser.contentWindow == win.gBrowser.selectedBrowser.contentWindow, etc.)

>+     createTabAtIndexInWindow : function(index, window_index) {
>+       let win = this.getWindow(window_index);
>+       if (win != null) {
           let tab = win.gBrowser.addTab();
           win.gBrowser.moveTabTo(tab, index);

browser.js is loaded in each browser window, and you don't really need this code to run more than once, AFAICT. It should probably live in nsBrowserGlue (a singleton) instead.

Referring to windows by index is a bit odd. Our windows have unique numerical IDs now, could we use that instead? They're arbitrary numbers, so I guess that might be confusing, but I'm not really familiar with what's common in other Applescript APIs.
Comment 9 User image Scott Greenlay [:sgreenlay] 2010-11-12 14:31:36 PST
Created attachment 490241 [details] [diff] [review]
Applescript support (v1.2)

Moved to nsBrowserGlue.js, added some of gavin's fixes (the ones that work in the new location).
Comment 10 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2010-11-16 11:25:34 PST
Why didn't some of my suggestions work? I don't think any of them depended on the location of the code.
Comment 11 User image Scott Greenlay [:sgreenlay] 2010-11-16 11:30:11 PST
> Array.forEach(gBrowser.browsers, function (b) {
>   array.appendElement(b.contentWindow);
> });

Didn't work because there isn't a global gBrowser in nsBrowserGlue.js

> Shouldn't this use isFullBrowserWindow too?

isFullBrowserWindow will not work on XUL windows
Comment 12 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2010-11-22 01:22:49 PST
(In reply to comment #11)
> Didn't work because there isn't a global gBrowser in nsBrowserGlue.js

You need to use win.gBrowser as you did elsewhere in the patch.

> isFullBrowserWindow will not work on XUL windows

Er, isFullBrowserWindow is only useful on XUL windows... you use it on the result of the enumeration of "navigator:browser" in getWindow(), so it should work just as well in getWindows().
Comment 13 User image Mark Banner (:standard8) 2010-12-13 14:12:06 PST
I've only just seen this patch, but from a quick glance I'm concerned about how applications (xulrunner, non-Firefox) etc, would extend the applescript for their own uses.

For example Thunderbird would want to be able to use the applescript interface to send email. Is MacScripting.mm the only place this can happen, or would it be able to be extended from a non-toolkit directory?

The design document talks about implementing things in different applications, but not how the design of the code works.
Comment 14 User image Scott Greenlay [:sgreenlay] 2010-12-13 17:53:40 PST
The patch on this bug is to fix the Firefox-specific Applescript requirements (and to fix the bugs outlined at the beginning of the design docs). Since all the code lives in a component, you can choose whether or not to include it in your own xulrunner app.

This is separate from adding generic Applescript support, where we are still exploring the available options. Further discussion on how we can create an extensible system that allows xulrunner apps to register for Applescript calls should happen in bug 61356.
Comment 15 User image Scott Greenlay [:sgreenlay] 2010-12-14 14:59:53 PST
Created attachment 497635 [details] [diff] [review]
Applescript support (v1.3)
Comment 16 User image Scott Greenlay [:sgreenlay] 2010-12-14 15:02:12 PST
Created attachment 497636 [details]
Applescript sample (v1.1)
Comment 17 User image Joe Hruska 2011-02-15 13:45:46 PST
Any updates on when we might see these changes incorporated into a Firefox release?  The last update on the wiki ( https://wiki.mozilla.org/Mac:AppleScript ) was over 3 months ago.
Comment 18 User image Scott Greenlay [:sgreenlay] 2011-03-28 09:15:01 PDT
Comment on attachment 497635 [details] [diff] [review]
Applescript support (v1.3)

No longer applies cleanly to trunk
Comment 19 User image Scott Greenlay [:sgreenlay] 2011-04-14 21:02:11 PDT
Created attachment 526180 [details] [diff] [review]
Applescript support (v1.4)

Updated to apply cleanly on trunk.
Comment 20 User image Jason Grimes 2011-07-13 15:29:35 PDT
(In reply to comment #19)
> Created attachment 526180 [details] [diff] [review] [review]
> Applescript support (v1.4)
> 
> Updated to apply cleanly on trunk.

Hey guys,
Any update on when we might see this in a release? We've got some software that requires this fix.

Thanks in advance,
Jason
Comment 21 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2011-08-01 17:49:59 PDT
Comment on attachment 526180 [details] [diff] [review]
Applescript support (v1.4)

I'm sorry that I started off by focusing on specifics here - I think what this bug needs more than anything is a more general revisiting of the implementation strategy and API, and someone who can work on this actively. It appears that sgreenlay is not available to do that, at the moment, so there's no use leaving this sitting in my review queue.
Comment 22 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2013-02-27 08:29:23 PST
*** Bug 237733 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.