448454 - Add support for Apple Remote

Status: NEW

(Core :: DOM: Events, enhancement, P5)

Description

[Jesse Ruderman]

I'd like to be able to switch slides in S5 presentations using the Apple Remote.  I'm imagining that Firefox would send DOM events to the focused web page (if the user has set a pref? if the page is full screen?).

Rob Arnold has expressed interest in working on this.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

I have no idea what kinds of (native) events Apple Remote sends, but when
translating those to DOM Events, DOMCommandEvent might be useful.
We use that to handle some special command keys, see bug 360731.

Or

if Apple Remote generates events which could be interpreted as keyboard events, 
perhaps creating VK_UP/DOWN/LEFT/RIGHT/etc key events is better.

Comment 2

[Tom Dyas]

MIT-licensed code to interface with Apple Remote (and other Mac remote controls apparently) is at: http://martinkahr.com/source-code/

Description of API at: http://martinkahr.com/2007/07/26/remote-control-wrapper-20/index.html

Comment 3

[Tom Dyas]

Attachment: Support Apple Remote buttons (for discussion purposes)

This patch is a first attempt at integrating the Apple Remote support library linked in comment #2.  The support library was very easy to integrate and detects apple remote events.  I get crashes when I try to inject key events into Cocoa, however, which has nothing to do with the support library, but rather my bride code.

A few issues I'd like people to weigh in on:

1.  The code is licensed under an MIT-style license.  Will this be a problem?

2.  A decision needs to be made as to how the remote control buttons will be presented to Gecko (transparent translation to key presses, inject a form of "AppCommandEvent", etc.)  Should this be configurable via preferences?

3.  Can a Cocoa guru tell me what I am doing wrong with my attempt to inject key press events in nsAppShell.mm?  The obj-c exception is:[NSRangeExcep
tion: *** -[NSCFString characterAtIndex:]: Range or index out of bounds]  This makes me think something is wrong with the two strings passed in to the NSEvent to create each event.

4.  Should the files for the remote control support library be moved to their own subdirectory?

Comment 4

[timeless]

Comment on attachment 351133 [details] [diff] [review]
Support Apple Remote buttons (for discussion purposes)

i don't think MIT is a problem
typically we don't use tabs
typically we don't allow trailing whitespace
typically we don't use more than 80 chars per line

is there a CVS/svn/hg manager for the MIT code, i.e. some sort of proper 'upstream'

+	The class is not thread safe

This class ...

+- (void) setCookieMappingInDictionary: (NSMutableDictionary*) 
+		// 10.5.x Leopard

what about 10.6?

+		// There is no seperate

separate 

+ * THE SOFTWARE IS PROVIDED “AS IS”,

something odd here, the earlier files have some different encoding because i see the quotes there as garbage.

++ (const char*) remoteControlDeviceName {
+	return "";
+}
+
++ (BOOL) isRemoteAvailable {	
+	io_object_t hidDevice = [self findRemoteDevice];
+	if (hidDevice != 0) {
+		IOObjectRelease(hidDevice);
+		return YES;
+	} else {
+		return NO;		
+	}
+}

if this were our code, we'd write:

+ (BOOL) isRemoteAvailable {	
	io_object_t hidDevice = [self findRemoteDevice];
	if (!hidDevice != 0) {
		return NO;		
	}
	IOObjectRelease(hidDevice);
	return YES;
}

+	// A security update in february

February 

+	// As there is no official Apple Remote API from Apple I also failed to open a technical incident on this.

i.e. no radar: url, gah.

+- (IBAction) stopListening: (id) sender {
+	if ([self isListeningToRemote]==NO) return;

this file generally uses spaces around !=, i'd expect the same for ==, at least in gecko code...

+	if (cookieString == nil || [cookieString length] == 0) return nil;

+		// let's see if a number of events are stored in the cookie string. this does
+		// happen when the main thread is too busy to handle all incoming events in time.

does happen => happens
in time => on time

+		NSString* lastSubCookieString=nil;
this code generally sticks spaces around =, 

+		while(subCookieString = [self validCookieSubstring: cookieString]) {

gecko code generally uses a space between 'while/if' and '('

like this:
+		if (processesBacklog == NO && lastSubCookieString != nil) {

+			// process the last event of the backlog and assume that the button is not pressed down any longer.

not pressed down any longer => no longer depressed

+		while (element = [elementsEnumerator nextObject]) 

note the space after while :)

+// notifaction names that are being used to signal that an application wants to 

notification ?

Comment 5

[Tom Dyas]

There isn't a "proper" upstream in the sense of ongoing development out of a publicly accessible source code control system.  The code is used by other open source projects, however, including VLC (the media player) and Open Office (see http://svn.services.openoffice.org/opengrok/xref/DEV300_m36/apple_remote/).  Open Office didn't go so far as to modify spelling errors and white space it seems despite some header comments saying it was adapted for use with Open Office.

I'd rather just isolate this code in a subdirectory of widget/src/cocoa and insert some comments at the top of each file saying where it came from and that it a third-party library that does not conform to Gecko coding guidelines. This isn't code that needed significant modification to work with Firefox; indeed, it required no modification whatsoever.

For now, I'd like to resolve the substantive issue of what kind of events a remote control button press should inject into Gecko.  Thoughts?

Comment 6

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #5)

> For now, I'd like to resolve the substantive issue of what kind of
> events a remote control button press should inject into Gecko.
> Thoughts?

I'll get to this eventually (a few weeks?).

Unless someone else beats me to it :-)

Comment 7

[Tom Dyas]

Safe to say that this is low priority!

Comment 8

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 351133 [details] [diff] [review]
Support Apple Remote buttons (for discussion purposes)

I'm assigning myself to review this code.

I won't get to it for a while, but I don't want to lose track of this
bug.

Comment 9

[Tom Dyas]

Attachment: Support Apple Remote buttons (for discussion purposes) V2

Everything (including actions to trigger) is now configurable via preferences.  Support is disabled by default. Set "ui.mac.appleremote.enabled" to true to enable it (no need to restart application).  App commands can be triggered by setting the applicable "action" preference to "appcmd:CMD" where CMD is the app command.

The third-party library is now isolated in widget/src/appleremote.  All of the code interfacing it to widget library is in nsAppleRemoteSupport.mm.

TODO (after discussion):
* Add support for generating key press events into Gecko.  All that needs to be done is to parse the action preference and inject the event.

Comment 10

[Tom Dyas]

Comment on attachment 351491 [details] [diff] [review]
Support Apple Remote buttons (for discussion purposes) V2

Adding [e-mail] back as reviewer since I made the previous patch obsolete!

Comment 11

[Tom Dyas]

Attachment: Support Apple Remote buttons (for discussion purposes) V2.1

Forgot to "hg qrefresh" before uploading the patch.  Ho hum ...

Comment 12

[Josh Aas]

This seems best done as an extension for now. I don't think appleremote->firefox usage numbers will validate pulling this into the tree.

Comment 13

[Jesse Ruderman]

I wonder how hard this is to do as an extension.

Comment 14

[Mike Beltzner [:beltzner, not reading bugmail]]

I actually think this would be a nice win for the product, and might not end up being OS X only as other OSes add support for remote access (media centers amongst them). Not sure that we should make the events so OSX specific in name.

Comment 15

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Unless I'm misreading this, this only allows the remote to give events to Firefox itself, which isn't all that useful.  More useful would be exposing those events to content, which enables using the remote to control things like audio and video players.

Comment 16

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 351492 [details] [diff] [review]
Support Apple Remote buttons (for discussion purposes) V2.1

Sorry, but I'm not going to be able to get to this anytime soon.

Comment 17

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.