127411 - Javascript generated "accesskey"'s are ignored.

Status: NEW

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Perry Lorier]

If you create/change an anchors accesskey attribute in javascript, the browser
will ignore it and use the old value (or ignore it if there was no previous value).

URL above shows a page where the first link has no accesskey, and the second
link has "2" as it's accesskey.  Javascript onload sets these to "0" and "1"
respectively.  

Pressing Alt+0 has no effect, expected: Should activate the first link.
Pressing Alt+1 has no effect, expected: Should activate the second link.
Pressing Alt+2 opens the second link, expected: No effect.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: The testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

It looks like nsHTMLAnchorElement::SetAccessKey never gets called.  I tried using

i.setAttribute("accesskey", j);

and 

i.accesskey = j;

in addition to the namedItem stuff in the testcase.  None of these work.

Comment 3

[Perry Lorier]

Attachment: Patch to allow javascript to update accesskeys.

This patch modifies nsHTMLAnchorElement::SetAttr by unregistering and then
registering the key when the accesskey is updated.

Comment 4

[Aaron Leventhal]

-> jag, for triage

Comment 5

[jag (Peter Annema)]

Triage? I thought this was your call? Either we need this, or we don't.

Anyway, as for the patch, you need to clean up your indentation. And think about
what happens when a user sets the accesskey through i.accessKey (Boris:
interCaps). It seems you can remove the support code from ::SetAccessKey and
just depend on ::SetAttr doing the right thing.

jst, hyatt, for HTML accesskeys, do we want these things to always be
registered, even if there's no frame (e.g. the link is hidden)? Could we also do
this for XUL then (if so, I can clean up the code for bug 959 quite a bit).

Comment 6

[Perry Lorier]

Attachment: Revised patch

> And think about what happens when a user sets the accesskey through
i.accessKey
> (Boris: interCaps).

Excuse my ignorance, but I don't understand this statement.  is "i.accessKey"
handled differently?

Comment 7

[jag (Peter Annema)]

My apologies, my comment wasn't clear.

.accessKey goes through ::SetAccessKey, which calls RegUnreg, SetAttr and RegUnreg:

http://lxr.mozilla.org/seamonkey/source/content/html/content/src/nsHTMLAnchorElement.cpp#245

You're fixing ::SetAttr to RegUnreg, so now if you call .accessKey you will
RegUnreg doubly.

I suspect (I apparently had this all worked out in my mind, judging by the
comment I made) that you can remove the RegUnreg code from ::SetAccessKey and
trust ::SetAttr to do the right thing.

Comment 8

[Aaron Leventhal]

Looks like I accidentally marked this WONTFIX.

Comment 9

[Marco Jiménez]

(In reply to comment #8)
> Looks like I accidentally marked this WONTFIX.

Hello,

     I've fund the same problem in Mozilla 1.0, on windows, with checkBoxes; for
example:
  
  document.formName.checkBoxName.acessKey = '1'.

   According this bug report is part of the core of mozilla, that's probably why
i got the same.

Reading the revised path, I read this line: 

-  if (aName == nsHTMLAtoms::href && kNameSpaceID_None == aNameSpaceID) {

I don't known about the libraries of mozilla but I think it only will work with
href not with other tags like the checkBoxes.

Please correct me if i'm wrong.

Thank you.

Comment 10

[Aaron Leventhal]

>   document.formName.checkBoxName.acessKey = '1'.
Try this spelling:
accessKey

Looks like you're missing a "c".

Comment 11

[Marco Jiménez]

You're right Aaron, it was a typing mistake, I've to make two corrections to my
post.

1. I found the bug not in Mozilla 1.0 it was in FireFox 1.0.
2. The example to test is: document.formName.checkBoxName.accessKey = '1'.

Thank you.



(In reply to comment #10)
> >   document.formName.checkBoxName.acessKey = '1'.
> Try this spelling:
> accessKey
> 
> Looks like you're missing a "c".

Comment 12

[Wayne Mery (:wsmwk)]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050307
Firefox/1.0+

testcase WFM on XP. 
if broke in linux, why not check in the patch?

Comment 13

[Stefan Seifert]

Attachment: button testcase

The link testcase WFM on Linux, but the problem still exists with elements other than links, namely buttons. I stumbled over this when developing an in-house application where keyboard usage would be much more efficient.

Modified the testcase to show the button problem and attached it.

Comment 14

[Robert Flaherty]

I just browsed the code briefly on mxr.mozilla.org, and it looks like each of these overrides BindToTree/UnbindFromTree/SetAttr/UnsetAttr:

nsHTMLAnchorElement.cpp
nsHTMLAreaElement.cpp
nsHTMLLabelElement.cpp
nsHTMLLegendElement.cpp

It seems like the RegUnRegAccessKey calls should just be pushed down into nsGenericHTMLElement.  I don't know if the code has a performance hit, but it seems like it would be less error prone.