Steps to reproduce: 1. Open prefs. 2. Leave focus in the category tree, or focus one of the ok/cancel/help buttons. 3. Alt+A to focus the location field. Result: Mozilla beeps. Expected: Focus should move to the location field. See also bug 64606, "page accesskeys don't work when location bar has focus".

030808 trunk build for MacOS9 seems WFM.

I believe, you meant Alt+L to focus the location field, not Alt+A (L is underlined). I also see that other mnemonic Alt+R to activate 'Restore Default' button - doesn't work as well.

i see this on linux as well [2002.03.13.08 comm bits]. olga, jesse is correct --in the Navigator preferences panel, the mnemonic for Location is the (underlined) 'a'. workaround: hit tab to move focus from the category to the pref panel, then the accesskeys work. Aaron, this shouldn't be necessary, should it? if so, perhaps this should be nominated?

clarifying summary a bit --when focus is in either the category item or the main OK / Cancel / Help window buttons, the dialog accesskeys won't work.

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

Changing summary to catch dupes.

Assigning to Gilbert Fang - this is just like bug 130644. Keystrokes should be made available to all iframe's in a dialog, if they're not first handled by the focused iframe. CC'ing Bryner and Saari for comment on the proposed fix.

okay, aaronl. that is my pleasure.

the underlying reason is (from http://www.mozilla.org/projects/ui/accessibility/mozkeyintro.html) # Accesskey: an attribute in HTML or XUL for defining an *accelator*. ... and # Accelerator: a key who's behavior is defined locally by the widget the *focus* is currently in. But from the user's view, that is not convenient. I think we should change the accesskey's definition to : an attribute in HTML or XUL for defining an specail *accelator* which effective on the focus *window*. .... For some scenario, i suggest that 1. menuitem accesskey, as current, only effecive when the menus pop up. 2. xul-widget accesskey, only effective when they are not hidden. I think it is almost the current status. For eg. in pref dialog, only the iframe showing should has effective accesskey. 3. if there are conflicts, i suggest the priorities should be , from highest to lowest, focused widget, focused widget's sibling, focused widget's container/parent 's siblings, etc.. , focused window. Note: rule 2 and rule3 should satisfy the bug 64606. Hi, how about others' suggestion?

For the implementation of my suggestion, one of the ways is to make all the accesskey registered to the top-level window. But i do not think it is a good idea, because then the top-level window should care too much things, initially finding all the accesskey from all the sub domxul node including their the bindings, listening the dom change event because maybe some codes dynamically appending accesskey attributes to a domsul node, anyway , I am not so sure about this. The other way is we do some event bubble-down with most carefulness. I suggest adding a new event handling state--NS_EVENT_FLAG_EXPANDING It will cause a little performance issuse, but i think it will not cost too much if we only expanding key events with the IsAlt== true. mouse event and other events do not need expanding.

oh, pls forgetting what i said in Comment #10. Now, I think it should be handled in the ESM::PreHandleEvent. After directly the local ESM processing the accesskey, it is a good place to check all his sub docShell to see whether the key press is an accesskey.

Gilbert, does this handle when there are more than 2 levels deep of docshells? For example, docshell / \ docshell docshell / \ / \ ds ds ds ds The event can occur on any one of those, right?

yes, aaronl. that is like a recursive process. every ESM checks all its docshell's sub docshells. so it can traversing into any level of doc shell.

oh, sorry, aaronl. if you mean the event is in sub docshell 2 levels deeper. then it may be not.

Maybe I'm wrong, but I think the event can occur in any of the docshells. docshell / \ docshell docshell / \ / \ *ds ds ds ds* <-- accesskey needs to be processed here ^ | accesskey event occurs here

i am not sure. what i think is the presShell will handle the event firstly, and make the pass the key press event to the top-level docshell's ESM . for example, open a new brower window, ctrl-T to create at least 2 tabs. then in one tab, go to resource:///res/samples/test9.html, make the focus in the frame1. Then alt-T still works, so i think there is already some mechanism to pass the event to the top-level docshell's ESM.

Comment on attachment 95820 [details] [diff] [review] checking all sub docshell's accesskey >+ nsCOMPtr<nsIDocShellTreeItem> subShellItem; >+ docShell->GetChildAt(counter, getter_AddRefs(subShellItem)); >+ nsCOMPtr<nsIDocShell> subDS = do_QueryInterface(subShellItem); >+ if (subDS) { >+ nsCOMPtr<nsIPresShell> subPS; >+ subDS->GetPresShell(getter_AddRefs(subPS)); >+ >+ nsCOMPtr<nsIPresContext> subPC; It is better to change if (subDS) { to if (subDS && IsShellVisible(subDS)) {

hi, joki and saari would u please review the patch?

need r=?

The following is from bryner on IRC today <bryner> this patch is pretty scary <bryner> i've got a couple of concerns here: <bryner> 1. i want to make sure this doesn't slow down typing <bryner> 2. i'd prefer if the subdocument ESM was only asked to do the accesskey handling, as opposed to all the stuff PreHandleEvent does. functionally, they should be the same, but i'd prefer if the accesskey handling was factored out of PreHandleEvent into its own method, and you just call that method on the subdoc ESM. <bryner> that way we don't risk extra stuff happening as a result of calling PreHandleEvent

But I donot think it is really scary. 1#. my codes is processed only when accesskey happens --- keypress event with accesskey modifier is true. And in my w2k box, mozilla has no apparently affected in speed at all. And in most case, docShell is less than 4 levels or even less. 2# That is really a good solution. The PreHandleEvent is handling too much things. If we factor out the accesskey handling by its own method, then we have to change the *idl*. Would that be Okay, bryner?

Well, actually nsIEventStateManager is not an idl interface. But you could also just put it as a protected method on nsEventStateMangaer, since you're doing this from inside the same class, and just cast the subdocument's ESM to nsEventStateManager*.

need r=?

Comment on attachment 96669 [details] [diff] [review] factor out the HandleAccessKey from nsEventStateManager >+void >+nsEventStateManager::HandleAccessKey(nsIPresContext* aPresContext, nsKeyEvent *aEvent, nsEventStatus* aStatus) >+{ >+ //This method is experimental, so here is no strict parameter checking. >+ //and it is supposed that this method is only called in PreHandleEvent after checking the access >+ //modifier key is down. >+ What exactly do you mean that this is "experimental"? That part of the comment is confusing. Also, the prevailing style for // comments is to have a space between '/' and the beginning of the comment. >+ >+ nsIEventStateManager * iesm = subESM; >+ nsEventStateManager * esm = NS_STATIC_CAST(nsEventStateManager *, iesm); I think you can do without the temporary here: nsEventStateManager* esm = NS_STATIC_CAST(nsEventStateManager*, subESM.get()); I can't remember offhand if that actually works though, so you should test it. >+ if (esm) { >+ esm->HandleAccessKey(subPC, aEvent, aStatus); >+ } >+ >+ if (nsEventStatus_eConsumeNoDefault == *aStatus) { >+ break; >+ } Please be consistent with whether single-line |if| bodies are made into blocks. r=bryner with those fixes.

Comment on attachment 96970 [details] [diff] [review] new patch after bryner's review Carrying bryner's review

+ //Alt or other accesskey modifier is down, we may need to do an accesskey + //Someone registered an accesskey. Find and activate it. + //B) Click on it if the users prefs indicate to do so. I know you just copied those, but please add spaces after the "//" + // after the local accesskey handling Extra space after "local" + //checking all sub docshell; Space after "//"? "sub docshells", remove the ';' + nsCOMPtr<nsIDocShellTreeNode> docShell(do_QueryInterface(pcContainer)); Assert of |docShell| is null after this? + nsCOMPtr<nsIPresShell> subPS; You never use this anywhere... Looking at this code, you're checking the children of the docshell. So if you have: A / \ 1 2 / \ / \ a b c d And an event happens in "2", the code will check for accesskeys in "2", "c", and "d" but not in "A", "1", "a", or "b", correct? That does not match comment 9's list... Or is PreHandleEvent called on the ESM of "A" as well as on the ESM of "2"?

This testcase is for A / \ 1 2 / \ / \ a b c d

I had thought that the PreHandleEvent was called on the ESM of "A" as well as on the ESM of "2". see comment 11. But That is not true, and it seems menu accesskey handling is in a different part. Anyway, This testcase http://bugzilla.mozilla.org/attachment.cgi?id=97491&action=view can exam comment 28 and what i said in comment 9.

Comment on attachment 97492 [details] [diff] [review] this patch will bubble up the proecessing and traversing the docShell tree >+ // Alt or other accesskey modifier is down, we may need to do an accesskey >+ // Someone registered an accesskey. Find and activate it. >+ // B) Click on it if the users prefs indicate to do so. + // after the local accesskey handling + // checking all sub docshells + nsCOMPtr<nsIDocShellTreeNode> docShell(do_QueryInterface(pcContainer)); + NS_ASSERTION(docShell, "no docShellTreeNode for presContext"); Yes, I had change that. >+ nsCOMPtr<nsIPresShell> subPS; >You never use this anywhere... No, It is used at + subDS->GetPresShell(getter_AddRefs(subPS)); + + subPS->GetPresContext(getter_AddRefs(subPC)); + + subPC->GetEventStateManager(getter_AddRefs(subESM));

Comment on attachment 97492 [details] [diff] [review] this patch will bubble up the proecessing and traversing the docShell tree sr=bzbarsky. Looks good!

Comment on attachment 97492 [details] [diff] [review] this patch will bubble up the proecessing and traversing the docShell tree check in

patch had been checked in and mozilla works well with my own sandbox on windows

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

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

This caused regression bug 167038

verification of this is blocked by bug 169767.

Bug 169767 only blocks verification of this when the pref is set to text field only, right? At least, I can tab into prefs panels if I set my accessibility.tabfocus to 7.

vrfy'd fixed with 2002.10.15.08 comm trunk builds on win2k and linux rh7.2 (access keys not supported on mac os).