608049 - Add Applescript Support to Firefox

Status: RESOLVED DUPLICATE of bug 125419

(Firefox :: Shell Integration, defect)

Description

[Scott Greenlay [:sgreenlay]]

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

[Scott Greenlay [:sgreenlay]]

Attachment: 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

[Scott Greenlay [:sgreenlay]]

Attachment: 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

[Scott Greenlay [:sgreenlay]]

Attachment: Applescript WIP (v1.1)

Small update to fix collisions with IPC code.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

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

[Scott Greenlay [:sgreenlay]]

Attachment: Applescript support (v1.0)

Comment 6

[Josh Matthews [:jdm]]

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

[Scott Greenlay [:sgreenlay]]

Attachment: Applescript support (v1.1)

Comment 8

[:Gavin Sharp [email: [e-mail]]]

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

[Scott Greenlay [:sgreenlay]]

Attachment: Applescript support (v1.2)

Moved to nsBrowserGlue.js, added some of gavin's fixes (the ones that work in the new location).

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Why didn't some of my suggestions work? I don't think any of them depended on the location of the code.

Comment 11

[Scott Greenlay [:sgreenlay]]

> 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

[:Gavin Sharp [email: [e-mail]]]

(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

[Mark Banner (:standard8)]

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

[Scott Greenlay [:sgreenlay]]

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

[Scott Greenlay [:sgreenlay]]

Attachment: Applescript support (v1.3)

Comment 16

[Scott Greenlay [:sgreenlay]]

Attachment: Applescript sample (v1.1)

Comment 17

[Joe Hruska]

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

[Scott Greenlay [:sgreenlay]]

Comment on attachment 497635 [details] [diff] [review]
Applescript support (v1.3)

No longer applies cleanly to trunk

Comment 19

[Scott Greenlay [:sgreenlay]]

Attachment: Applescript support (v1.4)

Updated to apply cleanly on trunk.

Comment 20

[Jason Grimes]

(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

[:Gavin Sharp [email: [e-mail]]]

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 23

[Mark Banner (:standard8)]

Consolidating this down into bug 125419 since that seems to be the original meta bug for all of this.