Closed Bug 220187 Opened 21 years ago Closed 20 years ago

TABINDEX property ignored for input type=file

Categories

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

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: spam, Assigned: nian.liu)

References

Details

(Keywords: testcase)

Attachments

(2 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20030924 Firebird/0.7+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20030924 Firebird/0.7+

The TABINDEX property for an input element type=file is ignored when tabbing
through the fields. Use the following snippit as an example:

<html>
<body>
<form action="test.html">
Field 1: <input type="text" tabindex="1" name="field1" /><br />
Field 2: <input type="text" tabindex="2" name="field2" /><br />
Field 3: <input type="text" tabindex="3" name="field3" /><br />
Field 4: <input type="file" tabindex="4" name="filename" /><br />
Field 5: <input type="text" tabindex="5" name="field5" /><br />
Field 6: <input type="text" tabindex="6" name="field6" /><br />
Field 7: <input type="text" tabindex="7" name="field7" /><br />
<input type="submit" tabindex="8" /><br />
</form>
</body>
</html>

Reproducible: Always

Steps to Reproduce:
1. Start in the first box.
2. Tab through the boxes. Notice how the file box is skipped until the end


Actual Results:  
Skips the input type=file box

Expected Results:  
Firebird should honor the tabindex value.

This happens with the latest nightly (as og 09/24/2003) and Firebird 0.6.1
This is not Firebird specific.  Also occurs in Mozilla 2003091704 PC/Win2K

Will attach a testcase.

->Browser
Assignee: blake → aaronlev5
Status: UNCONFIRMED → NEW
Component: General → Keyboard: Navigation
Ever confirmed: true
Keywords: testcase
Product: Firebird → Browser
QA Contact: sairuh
Version: unspecified → Trunk
Attached file testcase
Observerd on ix86 Linux as well (Mozilla/5.0 (X11; U; Linux i686; en-US;
rv:1.7a) Gecko/20040218). Platform=All?
Attached patch patch for 220187 (obsolete) — Splinter Review
input element with type file has two children which should have same tabindex
with their parent.
Comment on attachment 146067 [details] [diff] [review]
patch for 220187

>Index: nsFileControlFrame.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/layout/html/forms/src/nsFileControlFrame.cpp,v
>retrieving revision 3.154
>diff -u -r3.154 nsFileControlFrame.cpp
>--- nsFileControlFrame.cpp	19 Feb 2004 22:13:47 -0000	3.154
>+++ nsFileControlFrame.cpp	14 Apr 2004 02:17:16 -0000
>@@ -167,6 +167,11 @@
>         nsAutoString value;
>         fileContent->GetValue(value);
>         textControl->SetValue(value);
>+      //220187
>+      PRInt32 tabIndex = -1;
>+      fileContent->GetTabIndex(&tabIndex);
>+      textControl->SetTabIndex(tabIndex);
>+      //~220187

Please do not use comments like this, unless your fix is a hacking or
workaround. Also, there are some indent problem in your patch, please make them
consistent with the other part of the file.
Attachment #146067 - Flags: review-
Attached patch patch for 220187Splinter Review
modification based on previous patch due to coding style issues
Attachment #146067 - Attachment is obsolete: true
Attachment #146079 - Flags: review?(aaronleventhal)
Comment on attachment 146079 [details] [diff] [review]
patch for 220187

I downloaded and tested your patch with an additional test cases. It works
great. Thank you for fixing it. r=aaronlev
Attachment #146079 - Flags: review?(aaronleventhal) → review+
Attachment #146079 - Flags: superreview?(jst)
Comment on attachment 146079 [details] [diff] [review]
patch for 220187

	 nsAutoString value;
	 fileContent->GetValue(value);
	 textControl->SetValue(value);
+	 PRInt32 tabIndex = -1;
+	 fileContent->GetTabIndex(&tabIndex);
+	 textControl->SetTabIndex(tabIndex);
       }
     }
     aChildList.AppendElement(mTextContent);
...
+      nsCOMPtr<nsIDOMHTMLInputElement> fileContent =
do_QueryInterface(mContent);

This last line of code already exists above in a different scope. Please break
this out so that fileContent is declared once and the same variable is used in
both places. And return an error code (NS_ERROR_UNEXPECTED) if you can't QI it
to nsIDOMHTMLInputElement, in stead od checking it for null in both places.

sr=jst with that change.

PS: Please add "diff -u9 -Np" to your .cvsrc (especially the 'p') to make your
diffs more readable.
Attachment #146079 - Flags: superreview?(jst) → superreview+
Assignee: aaronleventhal → neo.liu
Fix revised acrroding to jst's review comment and checked in.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
*** Bug 278445 has been marked as a duplicate of this bug. ***
*** Bug 302898 has been marked as a duplicate of this bug. ***
Component: Keyboard: Navigation → User events and focus handling
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: