Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Add Applescript Support to Firefox

NEW
Unassigned

Status

()

Firefox
Shell Integration
7 years ago
a month ago

People

(Reporter: sgreenlay, Unassigned)

Tracking

(Blocks: 4 bugs)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 7 obsolete attachments)

(Reporter)

Description

7 years ago
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
(Reporter)

Updated

7 years ago
Assignee: nobody → sgreenlay
(Reporter)

Updated

7 years ago
Blocks: 46407
(Reporter)

Updated

7 years ago
No longer blocks: 46407
(Reporter)

Updated

7 years ago
Blocks: 369901
(Reporter)

Updated

7 years ago
Blocks: 491005
(Reporter)

Updated

7 years ago
Blocks: 516502
(Reporter)

Updated

7 years ago
Blocks: 490830
No longer blocks: 491005
(Reporter)

Comment 1

7 years ago
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.
(Reporter)

Comment 2

7 years ago
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)
(Reporter)

Updated

7 years ago
Status: NEW → ASSIGNED
(Reporter)

Comment 3

7 years ago
Created attachment 488372 [details] [diff] [review]
Applescript WIP (v1.1)

Small update to fix collisions with IPC code.
Attachment #488066 - Attachment is obsolete: true
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.
(Reporter)

Comment 5

7 years ago
Created attachment 489011 [details] [diff] [review]
Applescript support (v1.0)
Attachment #488372 - Attachment is obsolete: true
Attachment #489011 - Flags: review?(joshmoz)

Comment 6

7 years ago
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.
(Reporter)

Comment 7

7 years ago
Created attachment 489322 [details] [diff] [review]
Applescript support (v1.1)
Attachment #489011 - Attachment is obsolete: true
Attachment #489322 - Flags: review?(joshmoz)
Attachment #489011 - Flags: review?(joshmoz)
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.
(Reporter)

Comment 9

7 years ago
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).
Attachment #489322 - Attachment is obsolete: true
Attachment #490241 - Flags: review?(joshmoz)
Attachment #489322 - Flags: review?(joshmoz)
Why didn't some of my suggestions work? I don't think any of them depended on the location of the code.
(Reporter)

Comment 11

7 years ago
> 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
(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().
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.
(Reporter)

Comment 14

7 years ago
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.
(Reporter)

Comment 15

7 years ago
Created attachment 497635 [details] [diff] [review]
Applescript support (v1.3)
Attachment #490241 - Attachment is obsolete: true
Attachment #497635 - Flags: review?(gavin.sharp)
Attachment #490241 - Flags: review?(joshmoz)
(Reporter)

Comment 16

7 years ago
Created attachment 497636 [details]
Applescript sample (v1.1)
Attachment #488067 - Attachment is obsolete: true
(Reporter)

Updated

7 years ago
Attachment #497636 - Attachment mime type: text/plain → application/octet-stream

Updated

7 years ago
Attachment #497636 - Attachment mime type: application/octet-stream → text/plain

Updated

7 years ago
Attachment #497636 - Attachment mime type: text/plain → application/octet-stream

Comment 17

7 years ago
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.
(Reporter)

Comment 18

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

No longer applies cleanly to trunk
Attachment #497635 - Flags: review?(gavin.sharp)
(Reporter)

Comment 19

6 years ago
Created attachment 526180 [details] [diff] [review]
Applescript support (v1.4)

Updated to apply cleanly on trunk.
Attachment #497635 - Attachment is obsolete: true
(Reporter)

Updated

6 years ago
Attachment #526180 - Flags: review?(gavin.sharp)

Comment 20

6 years ago
(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 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.
Attachment #526180 - Flags: review?(gavin.sharp)
Duplicate of this bug: 237733
Assignee: scott → nobody
Status: ASSIGNED → NEW

Updated

a month ago
Blocks: 939528
You need to log in before you can comment on or make changes to this bug.