287461 - Caret Browsing is hardbound to the F7 key and is unable to be overridden with javascript like F3, F5, etc.

Status: RESOLVED FIXED

(Firefox :: Keyboard Navigation, defect)

Description

[Lindsey]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050225 Firefox/1.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050225 Firefox/1.0.1

I have an application that uses the function keys F1 through F12 for navigation.
I am able to override F3, F5, etc, through use of the Javascript
preventDefault() option, but unfortunatly it seems that the F7 key, which is
bound to a Caret Browsing dialog and the caret browsing toggle, does not use
Javascript events. It is actually bound in browser.xbl as an XBL handler
(http://lxr.mozilla.org/aviary101branch/source/toolkit/content/widgets/browser.xml#780),
which means that, as far as I can tell, there is no way to override it and stop
it toggling caret browsing.

The best solution that I can think of would be to move the F7 button to use the
same Javascript events that F3, F5, etc use, thus allowing it to be overridden
if desired.

Reproducible: Always

Steps to Reproduce:
1. Rebind F7 by using a Javascript event listener bound to keydown.
2. Use preventDefault() to stop the event from opening a dialog and toggling
caret browsing.
3.

Actual Results:  
Nothing - the dialog and toggle still appear.

Expected Results:  
Allowed me to override F7 like I have done with F3, F5, etc, by using the
preventDefault() function.

Comment 1

[Aaron Leventhal]

The reason the key is in the binding is so that any user of <browser> gets that
keystroke for free. Otherwise developers are most likely to forget it, yet it is
an important keystroke for accessibility -- for example in help, view source,
even in Chatzilla content.

I have no problem with using a different strategy to implement F7 to solve the
problems discussed in this bug, but we should continue to support it. Sun
Microsystems, especially, uses this feature for their screen reader strategy.
Some users need the key to be able to select text without need for a mouse.

Comment 2

[Pete Collins (MDG)]

Why not just role out a browser binding in the moz app that doesn't have the F7
handler? 

Comment 3

[Lindsey]

(In reply to comment #2)
> Why not just role out a browser binding in the moz app that doesn't have the F7
> handler? 

Unfortunately this isn't something we're able to do with our application at the
moment. The (initial) goal is to be able to run it off a standard install of
Firefox from a web server with no need to install extensions, etc.

Who knows, our clients might even start using Firefox for general browsing, and
some of them might want to use F7 :)

Comment 4

[Brian King [:kinger]]

The proposal is not to remove this key, but to move it out of the binding.

What I propose is:
- move the key into one of the existing XUL keysets.
- move the JS to browser.js or a more appropriate utility file if there is one.

Aaron, would you consider a patch if I put one together?

Comment 5

[Aaron Leventhal]

Brian, it's okay with me. Request r= from [e-mail] and sr= from
[e-mail]

Comment 6

[Brian King [:kinger]]

Copying Ginn and Neil for feedback. Do you have any objections before I proceed
with a patch to this key being moved out of the binding?

My fix would likely be:
- move the key to <keyset id="mainKeyset"> (browser-sets.inc)
- Move the JS to browser.js

Comment 7

[[e-mail]]

You should be able to make the browser binding respect preventDefault():
* Put the handler in the system event group (see bug 225563).
* Check to see if preventDefault() was called on the event.

Comment 8

[Brian King [:kinger]]

Attachment: Look for preventDefault

Neil, you mean something like this (not tested yet)?

Comment 9

[[e-mail]]

Comment on attachment 181269 [details] [diff] [review]
Look for preventDefault

That seems to work, although I'd prefer to see the !event.isTrusted after the
|| to avoid confusion over precedence.

Comment 10

[[e-mail]]

Oh, and r+sr too for the xpfe version tweaked as per my comment.

Comment 11

[Brian King [:kinger]]

Attachment: Updated patch

Order in conditional is reversed here as requested.

Neil, if it's OK can you check it in please (I've tested), I don't have checkin
rights.

Comment 12

[[e-mail]]

Comment on attachment 181281 [details] [diff] [review]
Updated patch

You'll need approval, as the tree is currently frozen.

Comment 13

[Pete Collins (MDG)]

Brian, when the tree is open, I can check it in for you if need be ...

Comment 14

[Ginn Chen]

What about Mozilla Suite?
Are you going to fix it?

Comment 15

[Brian King [:kinger]]

(In reply to comment #14)
> What about Mozilla Suite?
> Are you going to fix it?

Good point. Updated patch coming ...

Comment 16

[Brian King [:kinger]]

Attachment: Include Mozilla Suite binding

Patch that now includes browser.xml of Mozilla Suite.

Comment 17

[[e-mail]]

Comment on attachment 181524 [details] [diff] [review]
Include Mozilla Suite binding

r+sr=me

Comment 18

[Brian King [:kinger]]

I'd be grateful if any of you fine people could check this in once the tree is
open again. Thanks!

Comment 19

[Brian King [:kinger]]

Seeking driver review for checkin to trunk.

Comment 20

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 181524 [details] [diff] [review]
Include Mozilla Suite binding

a=shaver

Comment 21

[Brian King [:kinger]]

Checked in.