Bug 58208 – hidden attribute on <input type=file /> suppresses the file selection dialog

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Darth]

Created attachment 88957 [details]
testcase

Downstream bug: http://code.google.com/p/chromium/issues/detail?id=78961
Test case attached. Will show file selection box in firefox, and nothing in webkit.

<input id=inp type=file hidden />
<button onclick="document.getElementById('inp').click();">Click me</button>

The onclick event is fired though, which can be tested with a simple alert in the input.

Looking at the spec, I do read
"Elements that are not hidden should not link to or refer to elements that are hidden."
http://dev.w3.org/html5/spec/Overview.html#the-hidden-attribute
Though unsure if this falls into that category.

[Shuvam Das]

As per my observation, in Firefox and IE, we are able to open file system though the "file" element is hidden. 

In spec, there is written "All HTML elements may have the hidden content attribute set."

[Shuvam Das]

As per my observation, in Firefox and IE, we are able to open file system though the "file" element is hidden. 

In spec, there is written "All HTML elements may have the hidden content attribute set."

So in Chrome and Safari, the behavior should be the same as expected.

[Adam Barth]

> Looking at the spec, I do read

That's conformance information for authors, not for user agents.

We probably should match Firefox and IE here.  Suppressing the dialog in this case isn't buying us any security.  (The control could just as easily be "hidden" by being positioned off-screen.)

There's some question about whether the click() method should work at all on this control, but it's behavior shouldn't be different depending on the hidden attribute (unless required by compatibility).

[Ryosuke Niwa]

FWIW, we also disable implicit submission when a button is hidden.

[Charles Pritchard]

(In reply to comment #3)
> There's some question about whether the click() method should work at all on this control, but it's behavior shouldn't be different depending on the hidden attribute (unless required by compatibility).

Mozilla has enabled the .click() method even when the control is otherwise hidden. Using opacity: 0 and setting width/height dimensions are what I usually do for WebKit, though webkit's pseudo-selectors are powerful enough to handle it with more grace.

I strongly suggest keeping click, to stay in line with Mozilla.

[Alexey Proskuryakov]

Duplicate of bug 53246?

[Rakesh]

Created attachment 124739 [details]
Proposed patch

Patch v1

[WebKit Review Bot]

Attachment 124739 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
First, rewinding head to replay your work on top of it...
Applying: Fix compilation errors on build-webkit --debug --no-workers on mac.
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/qt/Skipped
CONFLICT (content): Merge conflict in LayoutTests/platform/qt/Skipped
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 Fix compilation errors on build-webkit --debug --no-workers on mac.

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Ryosuke Niwa]

Comment on attachment 124739 [details]
Proposed patch

Is it possible to use a similar technique employed in input-file-not-open-without-gesture.html to automatically test this?

[Rakesh]

(In reply to comment #9)
> (From update of attachment 124739 [details])
> Is it possible to use a similar technique employed in input-file-not-open-without-gesture.html to automatically test this?

Thanks for taking time to see this.

I think it is not possible to automate this test as file dialog opens only when there is user gesture. In input-file-not-open-without-gesture.html the test is other way round where we don't want file dialog to open when not an user gesture. Please do suggest if there is an way to automate.

[Ryosuke Niwa]

(In reply to comment #10)
> I think it is not possible to automate this test as file dialog opens only when there is user gesture. In input-file-not-open-without-gesture.html the test is other way round where we don't want file dialog to open when not an user gesture. Please do suggest if there is an way to automate.

But you can use eventSender to generate that "gesture", right?

[Rakesh]

(In reply to comment #11)
> (In reply to comment #10)
> > I think it is not possible to automate this test as file dialog opens only when there is user gesture. In input-file-not-open-without-gesture.html the test is other way round where we don't want file dialog to open when not an user gesture. Please do suggest if there is an way to automate.
> 
> But you can use eventSender to generate that "gesture", right?

Yes, I tried using eventSender but the problem is that when file dialog shows up
the current focus is on file dialog and until the file dialog closes the test won't end.

As per my understanding even if we generate user event with eventSender and test that body does not get the click event, we need to close the native file open dialog some how so that test ends(I think sending escape key event through eventSender will not have effect on native dialog).

[Ryosuke Niwa]

(In reply to comment #12)
> Yes, I tried using eventSender but the problem is that when file dialog shows up
> the current focus is on file dialog and until the file dialog closes the test won't end.

input-file-not-open-without-gesture.html uses setTimeout for that. It schedules a call to notifyDone before opening the dialog.

> As per my understanding even if we generate user event with eventSender and test that body does not get the click event, we need to close the native file open dialog some how so that test ends(I think sending escape key event through eventSender will not have effect on native dialog).

So calling notifyDone in setTimeout won't work?

[Rakesh]

(In reply to comment #13)

> input-file-not-open-without-gesture.html uses setTimeout for that. It schedules a call to notifyDone before opening the dialog.

Yes, it does and it works fine as file dialog does not open in this test. input-file-not-open-without-gesture.html does timeout if the click happens through eventSender.
 
> So calling notifyDone in setTimeout won't work?

It does not work as file dialog is still open and the call FileInputType::handleDOMActivateEvent is not yet complete, I mean the stack has not yet unwinded.

[Alexey Proskuryakov]

The file dialog should not be appearing in DRT and WTR, similarly to how alert() does not start a modal loop. This might require changes in WebCore and test runners.

[Rakesh]

(In reply to comment #15)
> The file dialog should not be appearing in DRT and WTR, similarly to how alert() does not start a modal loop. This might require changes in WebCore and test runners.

What do you suggest, should we go ahead with this patch and for file dialog not be showing up in DRT and WTR we can file a new bug?

[Alexey Proskuryakov]

It would certainly be better to make this testable and add a test in the same patch. But I didn't override the r+, meaning that I don't object to landing the patch as is.

[Ryosuke Niwa]

Comment on attachment 124739 [details]
Proposed patch

Given that it's hard to automate the test, it seems fine to land the patch as is.

[WebKit Review Bot]

Comment on attachment 124739 [details]
Proposed patch

Clearing flags on attachment: 124739

Committed r106538: <http://trac.webkit.org/changeset/106538>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.