206376 - accesskey doesn't work when field type is "file"

Status: RESOLVED FIXED

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

Description

[Daniel Convissor]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4b) Gecko/20030506
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4b) Gecko/20030506

In forms used for submitting files, if a field is type="file" and that field has
an accesskey set, the browser doesn't jump to that field when hitting ALT-<LETTER>.


Reproducible: Always

Steps to Reproduce:
1. Go to the page shown above.
2. Hit ALT-O.

Actual Results:  
Nothing.

Expected Results:  
The cursor should move to the field labeled "Photo File."

Comment 1

[Bill Mason]

Verified 20030516 PC/WinXP
->Keyboard navigation

Comment 2

[Daniel Convissor]

The sample URI has been changed.

The new one is
http://www.bikesummer.org/2003/photos/bugzilla.php

The old one was
http://www.bikesummer.org/2003/photos/add.php

The old one is for public use on the website.  I changed that version to use
<label> elements to define the accesskey.  When using this syntax, the keyboard
navigation works as expected.  None the less, the keyboard navigation should
still work for cases where the accesskey attribute is used inside the
   <input type="file" />

Comment 3

[Nian Liu(n/a in a long time)]

Attachment: patch for 206376

Comment 4

[Aaron Leventhal]

Comment on attachment 150700 [details] [diff] [review]
patch for 206376

Conditional r=aaronl if you change
|if (rv == NS_OK)| to either |if (NS_SUCCEEDED(rv))| or just use |if
(fileContent)|
If you use the second option you won't need to get rv in the QI.

Your patch also works with this case:
<label accesskey="c" for="bar">File <u>c</u>ontrol 2:</label> <input
type="file" id="bar">
That might have worked before, but I wanted to note that it's another case --
sometimes the accesskey is on the label.

Comment 5

[Nian Liu(n/a in a long time)]

Attachment: test case for file accesskey

Aaron, my test case includes both scenario you mentioned and works fine. so
IMHO, what i need to is to modify the condition clause and that's it, right?
correct me if i miss/misunderstandard anything.

Comment 6

[Aaron Leventhal]

Yes it works. All you need to do is change the condition. You have r= with that.

Comment 7

[Nian Liu(n/a in a long time)]

Attachment: patch v2

modification of condition clause according Aaron's suggestion

Comment 8

[Nian Liu(n/a in a long time)]

Attachment: patch v3

sorry for miss "!"

Comment 9

[Nian Liu(n/a in a long time)]

Attachment: final patch

really sorry. need strong coffee

Comment 10

[Aaron Leventhal]

Neo, I'm sorry but it's better to use

if (NS_FAILED(rv))

instead of 

if (!NS_SUCCEEDED(rv))

When you have time you might want to look at
http://lxr.mozilla.org/seamonkey/source/xpcom/base/nsError.h

Comment 11

[Johnny Stenback (:jst)]

Comment on attachment 150902 [details] [diff] [review]
final patch

Yeah, use NS_FAILED()...

sr=jst

Comment 12

[Nian Liu(n/a in a long time)]

Aaron and jst, after a browse of nsDebug.h, i have a question that any reason
not use NS_ENSURE_SUCCESS(rv, rv)?

seems it both have warning message and return if failed.

Comment 13

[Aaron Leventhal]

(In reply to comment #12)
> Aaron and jst, after a browse of nsDebug.h, i have a question that any reason
> not use NS_ENSURE_SUCCESS(rv, rv)?
> 
> seems it both have warning message and return if failed.

Use |NS_ENSURE_SUCCESS(rv, rv)| if you want to log an error.
Use |if (NS_FAILED(rv)) { return rv; }|  if you don't

Comment 14

[Nian Liu(n/a in a long time)]

Attachment: patch checked in

condition clause change

Comment 15

[Aaron Leventhal]

Nian, when you check in a fix, mark the bug fixed :)

Comment 16

[Nian Liu(n/a in a long time)]

^_^