92858 - [FILE INPUT] can change the contents of a DISABLED <input type=file> by using keyboard navigation

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect, P2)

Description

[Madhur Bhatia]

build ID : 2001-07-27
OS : tested on win2000, linux redhat 7.1, and macOS 8.6

If i use the tab key to navigate through the form-control [FILE INPUT]and hit 
the enter key to the corresponding control (whether it is disabled or not) , i 
get a file upload dialog box and am able to select anmd upload a file to show in 
the control.

steps to reproduce:
1. open the testcase via the above mentioned URL
2. tab to the first control - this is an enabled [file input] control.
3. hit the enter key 

what happens ? -- the file upload dialog box will appear. select a file and say 
open. the file selected will be uploaded in the enabled <input file> control.

4. U will have to hit the key once or twice more -- so that the focus is removed 
from the enabled control.

actual:
=======
what happens? --- the next control is the disabled <input file> control. you 
will notice that there is no visible focus on this control. Hit the "Enter" key.

Again -- the file upload dialog box will appear. select a file and say open. the 
file selected will be uploaded in the disabled <input file> control. 

The disabled control seems to have a focus, though there is no visible evidence  
-- the carat sign does not flicker in this control.

expected:
=========
disabled controls are not supposed to get focus of any kind.

Comment 1

[Madhur Bhatia]

to rerpoduce this bug effectively, set the focus (cursor shows) on the enabled 
<input file> control. Hit the tab key ( the focus should be removed from this 
enabled control) till u do not see the cursor blinking in that control anymore.
Now repeat step number 4 as mentioned above. 

This is very much reproducible and should be considered for an immediate fix.
Happens on all platforms.

Comment 2

[rods (gone)]

I would argue that this is a focus bug. The focus should go to the file control. 
Here is a patch to fix this for now:

Index: nsFileControlFrame.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/html/forms/src/nsFileControlFrame.cpp,v
retrieving revision 3.112
diff -u -r3.112 nsFileControlFrame.cpp
--- nsFileControlFrame.cpp      2001/08/25 02:00:10     3.112
+++ nsFileControlFrame.cpp      2001/08/27 17:26:35
@@ -248,6 +248,10 @@
 nsresult
 nsFileControlFrame::MouseClick(nsIDOMEvent* aMouseEvent)
 {
+  if (nsFormFrame::GetDisabled(this)) {
+    return NS_OK;
+  }
+
   // only allow the left button
   nsCOMPtr<nsIDOMMouseEvent> mouseEvent = do_QueryInterface(aMouseEvent);
   if (mouseEvent) {

Comment 3

[rods (gone)]

moving to 0.9.6

Comment 4

[Madhur Bhatia]

still reproducable on build ID: 2001-10-16-05-0.9.4 (branch build)- win32

Comment 5

[rods (gone)]

Beyond the problem with the mouse is that the input text inside the file control
will end up getting focus. Focus is "suppose" to skip a disabled element. It
appears it isn't skipping it's children.

May this is really a combination focus bug and formm control bug. It the focus
manager could check to see if it is a "replaced" element then it should traverse
down into thes children.

So when the file control is disabled we have to keep the input (and maybe event
the button from getting focus.

John, feeel free to future this if it is deemed "not worthy" for .99 or 1.0

One test is to tab into it and hit return. 

Comment 6

[John Keiser (jkeiser)]

Maybe the best thing to do is to just propagate DISABLED to the button and
textbox.  I wonder if there are other attributes or properties that should be
propagated.

Moving to 1.0, seems important (data integrity).

Comment 7

[Kevin McCluskey (gone)]

Moving to Moz1.1. Engineers are overloaded with higher priority bugs.

Comment 8

[John Keiser (jkeiser)]

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

Comment 9

[John Keiser (jkeiser)]

The proper fix seems to me to set the anonymous content disabled when the file
content is set disabled.  This goes for readonly too.

Comment 10

[John Keiser (jkeiser)]

Attachment: testcase

This testcase tests file inputs with disabled initially and set with JS; and
tests setting size, since I am changing that in the patch

Comment 11

[John Keiser (jkeiser)]

Attachment: Patch

This patch propagates the disabled attribute to the input type=text and button.
 It also changes mBrowse and mTextContent to be nsIContent*'s for efficiency
purposes (a lot less QI's but all the same functionality).  Finally, it removes
an unnecessary StyleChangeReflow() when the size attribute is dynamically
changed.  Everything still reflows correctly.

Comment 12

[John Keiser (jkeiser)]

BTW, I did not fix readonly because it is explicitly defined in the spec as
applying only to input type="text" and "password".

Comment 13

[rods (gone)]

Comment on attachment 86192 [details] [diff] [review]
Patch

r=rods

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 86192 [details] [diff] [review]
Patch

sr=roc+moz

Comment 15

[John Keiser (jkeiser)]

Fix checked in to trunk.