Bug 5882

Summary: disabled type="file" element doesn't appear disabled
Product: WebKit Reporter: Alexei Svitkine <myrd>
Component: FormsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: mitz
Priority: P2 Keywords: HasReduction
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
URL: http://projectmagma.net/~myrdred/file_input.html
Attachments:
Description Flags
Testcase
none
proposed patch
eric: review+
Patch including testcase
none
Testcase now in manual tests darin: review+

Description Alexei Svitkine 2005-11-29 16:59:42 PST
Other browsers support the disabling of input type="file" elements, either in the html directly 
( disabled="true" ) or with Javascript ( document.getElementById("file_input").disabled = true; ).

Neither the shipping nor the TOT version of WebKit handles either way properly, while all other browsers 
do.

See example with both Safari and other browsers at:
http://projectmagma.net/~myrdred/file_input.html
Comment 1 Michael J. Cohen 2006-01-10 09:21:46 PST
on r11976, the input type="file" element appears to be disabled functionally but does not appear 'greyed 
out'.
Comment 2 Joost de Valk (AlthA) 2006-02-08 11:24:04 PST
Seeing the same as MJC, changing subject, i will attached the testcase in the url to the bug itself.
Comment 3 Joost de Valk (AlthA) 2006-02-08 11:24:25 PST
Created attachment 6344 [details]
Testcase
Comment 4 Rob Buis 2006-05-12 06:10:45 PDT
Created attachment 8264 [details]
proposed patch

My first attempt at fixing this. Be aware that I have zero objC experience :)
I can make a new patch including the given testcase if needed.
Cheers,

Rob.
Comment 5 Eric Seidel (no email) 2006-05-12 09:28:08 PDT
Comment on attachment 8264 [details]
proposed patch

This needs a test case, but otherwise looks fine.

Test cases are especially important for this area of the code because Adele is about to re-write this control using the engine instead of AppKit. :)
Comment 6 Rob Buis 2006-05-12 13:51:07 PDT
Created attachment 8270 [details]
Patch including testcase
Comment 7 Darin Adler 2006-05-12 20:49:35 PDT
Comment on attachment 8270 [details]
Patch including testcase

Since this test is a manual test rather than an automatic one, it should be in WebCore/manual-tests rather than in LayoutTests.
Comment 8 Rob Buis 2006-05-13 03:09:34 PDT
Created attachment 8278 [details]
Testcase now in manual tests

For your convenience, I moved the testcase :)
Cheers,

Rob.
Comment 9 Alexey Proskuryakov 2006-05-13 06:25:41 PDT
(In reply to comment #8)
> For your convenience, I moved the testcase :)

But isn't it an automated test case, actually? :)

The original state is disabled, so a pixel test would catch the chage.
Comment 10 Darin Adler 2006-05-13 08:48:28 PDT
Comment on attachment 8278 [details]
Testcase now in manual tests

Seems fine. This makes the file button look disabled. However, I think this does not disable all the relevant file button behavior. It's worth testing whether a disabled file button responds to clicks on the file name, drags, and other kinds of interaction that don't directly involve the "choose file" button.

Also, we may want to change the appearance to make the file icon and file name look disabled as well.

r=me
Comment 11 Darin Adler 2006-05-14 20:40:33 PDT
(In reply to comment #9)
> But isn't it an automated test case, actually? :)
> 
> The original state is disabled, so a pixel test would catch the chage.

Oops. OK. Alexey right, me wrong, I guess we should land the version in LayoutTests.
Comment 12 Darin Adler 2006-05-14 21:29:04 PDT
Committed revision 14370.