Closed Bug 1189437 Opened 9 years ago Closed 2 years ago

click() doesn't work on <input type=file> inside onmousedown

Categories

(Core :: DOM: Core & HTML, defect, P3)

defect

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: evanw, Unassigned)

References

()

Details

(Keywords: parity-chrome)

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.107 Safari/537.36

Steps to reproduce:

Calling click() only works from within onclick, not onmousedown. This works fine in Chrome, in Safari, and even in IE. This being broken prevents our whole file picker implementation from working and doesn't seem to have any security implications. Is there a reason why this is broken?

Repro link:
http://jsfiddle.net/v2aecb1z/1/

Repro HTML:
<input id="file-picker" type="file">
<script>

document.onmousedown = function() {
  document.getElementById('file-picker').click();
};

</script>
<p>Click this, which should open the file picker. Doesn't work in Firefox.</p>
This works on google chrome, still lets see what Jared Wein says
Severity: normal → major
QA Whiteboard: [bugday-20150727]
Flags: needinfo?(jaws)
Version: 39 Branch → Trunk
I think this is by design.

If you want to allow popup with mousedown, you should add mousedown to dom.popup_allowed_events  in about:config.
Component: Untriaged → DOM
Product: Firefox → Core
Andrea, is this something that we can fix for parity?
Flags: needinfo?(jaws) → needinfo?(amarchesini)
Whiteboard: [parity-Chrome]
The problem is not the filePicker but the PopupBlockedEvent. For FilePicker we don't set the popupWindowURI and this generated this error:

JavaScript error: chrome://global/content/browser-content.js, line 302: TypeError: ev.popupWindowURI is null

This happens because:

http://mxr.mozilla.org/mozilla-central/source/dom/html/HTMLInputElement.cpp#630
Flags: needinfo?(amarchesini)
Attached patch block.patchSplinter Review
bz, this patch doesn't seem to be enough because, also allowing popups, HTMLInputElement::IsPopupBlocked() receives openAbused from win->GetPopupControlState().
Attachment #8646435 - Flags: feedback?
Comment on attachment 8646435 [details] [diff] [review]
block.patch

smaug any feedback?
Attachment #8646435 - Flags: feedback? → feedback?(bugs)
Are there any updates here? I was optimistic when I came across this thread because this is currently affecting me. I hope progress hasn't stalled :)
Comment on attachment 8646435 [details] [diff] [review]
block.patch

I don't understand what this patch is about.

dom.popup_allowed_events doesn't by default contain mousedown. Maybe it should?
But one needs to look at the cvs blame to see why it doesn't have mousedown.
Attachment #8646435 - Flags: feedback?(bugs)
Bug 258499 added (and removed) mousedown form that pref. Can we consider to add it again?
Testing it with nightly, this issue is fixed having 'mousedown' in dom.popup_allowed_events.
Flags: needinfo?(bugs)
Can you please check if opening new windows works during mousedown in IE/Edge and Blink?
Flags: needinfo?(bugs)
(In reply to Andrea Marchesini (:baku) from comment #10)
> Bug 258499 added (and removed) mousedown form that pref. Can we consider to
> add it again?
> Testing it with nightly, this issue is fixed having 'mousedown' in
> dom.popup_allowed_events.

If you simply add mousedown to the pref, it triggers a new problem.
I.e., when you open the dialog even once by mousedown, Firefox is unable to operate with mouse anymore. All mouse operation will bring the dialog.
> If you simply add mousedown to the pref, it triggers a new problem.
> I.e., when you open the dialog even once by mousedown, Firefox is unable to
> operate with mouse anymore. All mouse operation will bring the dialog.

Correct. This is why I wrote the beginning of that patch.
(In reply to Olli Pettay [:smaug] (high review load, please consider other reviewers) from comment #12)
> Can you please check if opening new windows works during mousedown in
> IE/Edge and Blink?

Blink supports this feature. IE/Edge, I don't know.
I don't know how we want to proceed. We are actually implementing what the spec says. But we could ask for a change in the spec...
Should we maybe file a bug to the spec and move the discussion there?
Flags: needinfo?(bugs)
Or we could special case popup blocking here, and explicitly allow filepicker to show up.
But anyhow, a spec bug sounds good.
Flags: needinfo?(bugs)
Mass bug change to replace various 'parity' whiteboard flags with the new canonical keywords. (See bug 1443764 comment 13.)
Keywords: parity-chrome
Whiteboard: [parity-Chrome]
Priority: -- → P3
Component: DOM → DOM: Core & HTML

(In reply to Olli Pettay [:smaug] from comment #18)

Or we could special case popup blocking here, and explicitly allow
filepicker to show up.
But anyhow, a spec bug sounds good.

Is there any move concerning this?
I would like to open the filepicker programmaticaly in an Add-on but can't without activating popups on the whole browser.

Since this is an old issue and I wasn't able to reproduce the issue on my end I'm going to close it for now.

If the issue does still occur to someone, please feel free to re-open the report.

Status: UNCONFIRMED → RESOLVED
Closed: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: