This is a simple feature straight out of that 'other' browser: the ability to set tabbing to either jump to each link on page OR to only jump between text fields. If we really wanna go crazy, 'alt-tab' or some other key combo should do the alternate tab usage. I prefer the latter, but that's me. It should be an option. -Brett I'm sorry if this is a dupe...I couldn't find any matches in bugzilla.

confirming rfe

See bug 66285 for some related discussion.

-> Akkana

Setting milestone based on a hope that the backend for this already exists. If focus code has to be rewritten, it will take longer.

Bryner pointed me in the right direction. and I have a patch. We discussed possibly making a new attribute like -moz-user-focus to set in addition to -moz-user-focus, to make it stronger (or weaker); but the current tabbing code in nsEventStateManager isn't looking at -moz-user-focus, so I haven't done anything like that. Adding Simon to the cc list because I seem to remember him expressing interest in this issue in another bug. There are three levels: tab only to text controls; tab to any form control; tab to all form controls plus links. I have currently set the defaults this way: Windows: same as before (tab to everything including links). Unix: Same as 4.x, tab only to text controls. Mac: same as before, tab to everything, which is what OS X IE does -- I don't have a mac version of 4.x to check. iCab doesn't tab between elements at all, just between page-as-a-whole and urlbar.

Akkana, I think you should use enums for the tab focus model, it will make the code much easier to understand. It's not obvious what this does: else if (mTabFocusModel < 2 && nsHTMLAtoms::button==tag.get()) { At least add a comment so people don't break your code later.

CC'ing Sun Beijing accessibility team, since this would change the default tabbing behavior on their platform. Please let us know if this is going to affect you. I think this is a big change for Linux users, even though 4.x did it that way. I'm worried about people not knowing how to get around it, since the pref is not exposed in the UI. There are serious consequences for people who can't use the mouse. They will be unable to do anything unless they know enough to edit their prefs.js file, which most people aren't. I understand this is better for most users, but please convince me that this isn't a problem.

That's fair. I thought about doing enums but didn't because the code is more complicated with them and the prefs can't use them anyway. But they're probably better even so, and I've rewritten the patch to use them. I've also added comments; I think most of the unclearness of the code was there before I got there :-) but perhaps the comment for each clause will make the flow clearer. If nothing else, it'll help point out whether I've misunderstood the existing code.

Aaron suggested bryner should probably look at this since it affects focus handling and someone experienced with focus needs to review it.

I hope we do expose this pref at some point - if that's the case, would it hurt anything to add a pref listener for this? // text is always in the tab order Nit: when I read "text" I don't think of text inputs, I think of text nodes Why would would you want the default to be text inputs only on UNIX. How is it useful to skip past buttons and selects?

> Nit: when I read "text" I don't think of text inputs, I think of text nodes Good point. I've changed that comment to say "text controls are always in the tab order". > Why would would you want the default to be text inputs only on UNIX. How is it > useful to skip past buttons and selects? That's what we always did in the past on Unix netscape, and my guess is that it matches most people's habits: I tab to get to a text field because I'm about to type so I want my hands on the keyboard, but for things like popup menus I'm more likely to use the mouse so I don't need to be able to tab there, and having to tab through a lot of other form elements to get to the one text field is frustrating. I'm open to discussion if other Unix users disagree with me.

Comment on attachment 97260 [details] [diff] [review] Patch addressing Aaron's comments + // Tab focus models: 0 (default) focuses links and form elements, + // 1 focuses form elements but not links, 2 focuses only text controls. + enum nsTabFocusModel { + eTabFocus_linksFormsText=0, + eTabFocus_formsText=1, + eTabFocus_textOnly=2, + eTabFocus_unset=-1 + }; I almost wonder if this would be better as a bitfield... that would simplify some of the comparisons below. >@@ -263,6 +271,9 @@ > > // So we don't have to keep checking accessibility.browsewithcaret pref > PRBool mBrowseWithCaret; >+ >+ // Which types of elements are in the tab order? >+ PRInt32 mTabFocusModel; > This is going to be global, isn't it? I'd make it a static class variable. >+ // Tab focus mode is constant across all windows. >+ // It would be nicer if ESM had a prefs callback, >+ // so we could store this and change behavior when it changes. >+ // But as long as the pref isn't exposed, that doesn't matter. >+ // 0=form controls and links, 1=all form controls, 2=text controls. >+ if (mTabFocusModel == eTabFocus_unset) { >+ mTabFocusModel = eTabFocus_linksFormsText; >+ nsresult rv = getPrefService(); >+ if (NS_SUCCEEDED(rv)) >+ mPrefService->GetIntPref("accessibility.tabfocus", &mTabFocusModel); >+ } >+ ... and then we wouldn't have to fetch this pref again for each document. > child->GetTag(*getter_AddRefs(tag)); > nsCOMPtr<nsIDOMHTMLElement> htmlElement(do_QueryInterface(child)); > if (htmlElement) { >@@ -3280,7 +3293,14 @@ > > nsAutoString type; > nextInput->GetType(type); >- if (type.EqualsIgnoreCase("hidden")) { >+ if (type.EqualsIgnoreCase("text")) { >+ disabled = PR_FALSE; // textfields are always in the tab order >+ } >+ else if (mTabFocusModel == eTabFocus_textOnly) { >+ // tab focus model specifies only text, and we're not. >+ disabled = PR_TRUE; >+ } >+ else if (type.EqualsIgnoreCase("hidden")) { > hidden = PR_TRUE; > } > else if (type.EqualsIgnoreCase("file")) { >@@ -3288,13 +3308,17 @@ > } > } > } >- else if (nsHTMLAtoms::select==tag.get()) { >+ // Select counts as form but not as text >+ else if ((mTabFocusModel == eTabFocus_linksFormsText >+ || mTabFocusModel == eTabFocus_formsText) >+ && nsHTMLAtoms::select==tag.get()) { Go ahead and remove the unneeded .get() on tag while you're here (and other places where you're changing these lines). >Index: modules/libpref/src/unix/unix.js >=================================================================== >RCS file: /cvsroot/mozilla/modules/libpref/src/unix/unix.js,v >retrieving revision 3.74 >diff -u -r3.74 unix.js >--- modules/libpref/src/unix/unix.js 25 Jul 2002 05:10:46 -0000 3.74 >+++ modules/libpref/src/unix/unix.js 30 Aug 2002 01:30:57 -0000 >@@ -67,6 +67,10 @@ > > pref("browser.urlbar.clickSelectsAll", false); > >+// Tab focus models: 0 focuses links and form elements, >+// 1 focuses form elements but not links, 2 focuses only text controls. >+pref("accessibility.tabfocus", 2); >+ > // override double-click word selection behavior. > pref("layout.word_select.stop_at_punctuation", false); > I suspect this would be desired behavior for Mac as well, but you should talk to a Mac guy about it.

Most mac users would want the pref at '2' by default, I think.

Yeah, we would. That's why I reported this in the first place. ;-) Thanks for the effort, guys. -Brett

Transferring keywords from bug 66285 prior to duping it to this one. I'll attach a new patch soon with the changes bryner suggested (and with mac as well as unix defaulting to text fields).

Transferring dependency list too.

*** Bug 66285 has been marked as a duplicate of this bug. ***

Here's a new patch addressing Bryner's comments. This is actually much clearer -- I'm glad you suggested the bit fields, as it makes the code much more readable and also gives the user more control (not that anyone is likely to want to tab to links but not text fields, say, but should they want to, it works). I've removed all the .get()'s I saw, and I made mac and unix both default to text fields only, windows default to everything. Seeking review ...

Comment on attachment 99181 [details] [diff] [review] New patch addressing bryner's comments Usually in Mozilla we use #defines for bitfields, i.e.: #define TAB_FOCUS_TEXT_CONTROLS (1 << 0) #define TAB_FOCUS_FORM_ELEMENTS (1 << 1) #define TAB_FOCUS_LINKS (1 << 2) To me, that seems cleaner than a non-contiguous enum. Also, could I convince you to attach a patch with -w? :)

nominating.

I doublechecked with Simon since I'll want his sr (to me the enum seems cleaner but I don't feel strongly about it). He agreed about the (1<<n), but preferred an enum, partly because it guarantees that the compiler will handle it at compile time rather than runtime (though most modern compilers should calculate constants at compile time anyway), but he suggested cleaning it up by adding Mask to each of the enum symbols: eTabFocus_linksMask = (1 << 2) I hope that's okay with you, Brian. I've done that (I left the Mask part off unset, since that's a value, not a mask) and made a new patch with -uw as bryner requested.

Comment on attachment 99442 [details] [diff] [review] diff -uw, with "Mask" and "(1<<n)" r=bryner (or sr=, if you want)

Comment on attachment 99442 [details] [diff] [review] diff -uw, with "Mask" and "(1<<n)" sr=sfraser

It's in! Happy tabbing. :-)

a couple regressions due to this fix (on linux and mac; win32 okay): bug 169767: focus/tabbing broken in application windows bug 169756: password fields should be part of tab cycle

Hey, who broke my tabbing order?!??!

Do we have a bug for complaining that we don't like the new defaults? I use tabbing to access radio buttons and checkboxes all the time. (I'm assuming we can still tab to file controls.) I also use keyboard navigation for selects quite a bit.

check out bug 169767... there are also bug[s] for requesting a frontend to control these --see the dep list for bug 169045.

FOCUS_FORM_ELEMENTS is interesting to us

Bug 169767, for getting the pref exposed, is an important one. But no, we don't have a bug for complaining about the new defaults. Lets not use this one -- let's make this be the one that introduced the pref, and you can open a bug specifically for changing the pref if you think it's warranted, and add a note here since a lot of linux and mac users cc'ed here felt quite strongly about wanting it set the new way, while a few other people (but mostly windows users, I think) felt strongly the other way.

The new default is less usable in every scenario I use daily, especially in bugzilla. I can't tab to a submit button and hit space? I have to use my mouse to pick a select option for bug component? Then why did I care to tab among text inputs only? Half as much fun, then some mouse-pain to balance yin and yang? Asa's going to file a bug demanding the default be reverted to what we shipped in mozilla1.0: bug 170429. /be

vrfy'ing fixed. actually bug 169767 isn't to expose the pref, it's to make sure that the pref only affects web page content (in the browser window). check out bug 169045 for frontend pref work.

*** Bug 160434 has been marked as a duplicate of this bug. ***