Closed Bug 1584539 Opened 5 years ago Closed 5 years ago

Regression: clicking "open containing folder" button in the downloads panel (and perhaps also about:downloads) also launches/opens the downloaded file

Categories

(Firefox :: Downloads Panel, defect, P1)

71 Branch
Desktop
All
defect

Tracking

()

VERIFIED FIXED
Firefox 71
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox69 --- unaffected
firefox70 --- unaffected
firefox71 + verified

People

(Reporter: Gijs, Assigned: ckerschb)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #1582532 +++

From comment #2 on that bug:

STR
0. Associate ".zip" file to 7-zip etc..

Open http://ftp.mozilla.org/pub/firefox/nightly/2004/02/2004-02-10-08-trunk/MozillaFirebird-win32.zip
Save it.
Click the downloads button in NavBar after download finished.
Click Folder Icon at the right side of list item

Actual results:
File Explorer and zip associated application will open.

Expected Results:
Only File Explorer will open.

This is a regression from bug 1497200. The original bug 1582532 seems to be a separate issue.

[Tracking Requested - why for this release]:
This is a nightly regression.

Christoph, your patch in bug 1497200 seems to have caused this bug, could you have a look please? Thanks

Flags: needinfo?(ckerschb)

(In reply to Pascal Chevrel:pascalc from comment #2)

Christoph, your patch in bug 1497200 seems to have caused this bug, could you have a look please? Thanks

Yep, it's already assigned to me - I'll take care of it.

Flags: needinfo?(ckerschb)

Just to add, the issue is only in the downloads panel. The Library Downloads branch and the about:download page both working as expected

Hi Gijs,

I think I figured what the problem is, the original code used an "oncommand" handler [0]. Within onDownLoadClick [1] the event does not get treated if it has the attribute "oncommand". Question is, how do we fix that? Can we somehow check if an "command" event listener was added using .addEventListener? Or is there a better approach?

I fiddled with the code for testing purposes and if that code [1] is not executed then it works as expected, we only open the file explorer.

Any suggestions?

[0] https://hg.mozilla.org/mozilla-central/rev/1572223bd5ec#l1.13
[1] https://searchfox.org/mozilla-central/rev/f372e8a46ef7659ef61be9938ec2a3ea34d343c6/browser/components/downloads/content/downloads.js#819

Flags: needinfo?(gijskruitbosch+bugs)

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #6)

Hi Gijs,

I think I figured what the problem is, the original code used an "oncommand" handler [0]. Within onDownLoadClick [1] the event does not get treated if it has the attribute "oncommand".

Gah. This is horrible. :-(

Question is, how do we fix that? Can we somehow check if an "command" event listener was added using .addEventListener? Or is there a better approach?

I'm pretty sure event.originalTarget.localName == "button" should do the trick. Still ugly though. Might be worth adding a comment in the code that has the markup to indicate this could potentially be a problem if people changed the markup.

Other options involve also adding a click handler on the button and calling stopPropagation() in there, but that might prevent the command event being created, I'm not sure, and also wouldn't scale to trying to handle the command events more centrally if we wanted to avoid N handlers for N download items. That latter objection also applies to a third option, which would be to use the event listener service (Services.els.getListenerInfoFor); to ask the DOM whether there's a "command" event listener registerd on the originalTarget.

Flags: needinfo?(gijskruitbosch+bugs)

Also, it would be great if we could have tests for this, but I bet it's tricky to synthetically fire a "real" click event that demonstrates this problem...

See Also: → 1582532
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71
Flags: qe-verify+

Reproduced the initial issue on Firefox 71.0a1 (2019-09-27).
Verified fixed on Windows 7 x64, Ubuntu 18.04 x64 and macOS 10.15 using Firefox 71 Beta 5 (buildID: 20191028110005).

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: