Closed
Bug 1292566
Opened 6 years ago
Closed 6 years ago
The "open" button in the subview for temporarily blocked downloads should not ask for confirmation
Categories
(Firefox :: Downloads Panel, defect, P2)
Tracking
()
People
(Reporter: Paolo, Assigned: adw)
References
Details
(Keywords: regression, Whiteboard: [fxprivacy])
Attachments
(1 file)
58 bytes,
text/x-review-board-request
|
Paolo
:
review+
ritu
:
approval-mozilla-aurora+
|
Details |
The Downloads Panel subview for blocked downloads is designed as a replacement and not an addition to the previous modal confirmation dialog, so the "open" button should not ask for confirmation. Malware _may_ be a special case where an additional confirmation dialog can be wanted, we can clarify this with the designers. For uncommon and potentially unwanted downloads a modal dialog is surely not needed. The presence of this additional dialog is a regression in user experience, because we show the same message too many times.
Reporter | ||
Comment 1•6 years ago
|
||
[Tracking Requested - why for this release]: User interface regression with a new feature developed for Firefox 50 in bug 1252509.
tracking-firefox50:
--- → ?
Assignee | ||
Comment 2•6 years ago
|
||
How is this a regression? The feature didn't exist before 50.
Reporter | ||
Comment 3•6 years ago
|
||
The subview didn't exist, but malware download blocking was an existing feature. I've marked this explicitly as a regression so we don't lose track of it. It should be interpreted in the sense that the change to the existing feature inadvertently introduced a problem that didn't exist before, the repetition of the warning. Hope this clarifies!
Updated•6 years ago
|
Priority: -- → P2
Whiteboard: [fxprivacy][triage] → [fxprivacy][triage]
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → adw
Status: NEW → ASSIGNED
Updated•6 years ago
|
Whiteboard: [fxprivacy][triage] → [fxprivacy]
Comment hidden (mozreview-request) |
Reporter | ||
Comment 5•6 years ago
|
||
(In reply to Drew Willcoxon :adw from comment #4) > Review commit: https://reviewboard.mozilla.org/r/71352/diff/#index_header Drew, sorry for the delay on this review. I still have to look at the code in detail, but the overall comment is to make sure that the same modal confirmation dialogs are still shown when the blocked download is handled from the Library window, where the subview is not present, and from the context menu. You may need to add a different command to handle this.
Reporter | ||
Comment 6•6 years ago
|
||
mozreview-review |
Comment on attachment 8780712 [details] Bug 1292566 - The "open" button in the subview for uncommon and potentially unwanted downloads should not ask for confirmation. https://reviewboard.mozilla.org/r/71352/#review71018 Sorry again for the late review, just a couple of fixes needed. I'll take a look at the next iteration but I don't expect any special difficulty. ::: browser/components/downloads/DownloadsViewUI.jsm:298 (Diff revision 1) > verdict: this.download.error.reputationCheckVerdict, > window, > dialogType, > }).then(action => { > if (action == "open") { > - return this.download.unblock().then(() => this.downloadsCmd_open()); > + return this.openBlockedDownload(); "this.unblockAndOpenDownload()". Apparently we're not testing the dialog yet. ::: browser/components/downloads/content/downloads.js:1116 (Diff revision 1) > downloadsCmd_chooseUnblock() { > DownloadsPanel.hidePanel(); > this.confirmUnblock(window, "chooseUnblock"); > }, > > downloadsCmd_chooseOpen() { Rename to downloadsCmd_unblockAndOpen(), here and in downloadsOverlay.xul. The Library window will continue to use downloadsCmd_chooseOpen instead. ::: browser/components/downloads/content/downloads.js:1118 (Diff revision 1) > this.confirmUnblock(window, "chooseUnblock"); > }, > > downloadsCmd_chooseOpen() { > DownloadsPanel.hidePanel(); > - this.confirmUnblock(window, "chooseOpen"); > + this.unblockAndOpenDownload(); this.unblockAndOpenDownload().catch(Cu.reportError); ::: browser/components/downloads/test/browser/browser_downloads_panel_block.js:172 (Diff revision 1) > +function promiseUnblockAndOpenDownloadCalled(item) { > + return new Promise(resolve => { > + let realFn = item._shell.unblockAndOpenDownload; > + item._shell.unblockAndOpenDownload = () => { > + item._shell.unblockAndOpenDownload = realFn; > + resolve(); > + // unblockAndOpenDownload returns a promise (that's resolved when the file > + // is opened). > + return new Promise(r => r()); > + }; > + }); > +} When bug 1269962 lands you'll be able to use "sinon.stub", however this alternative is fine for now because we'll need to uplift the patch to Aurora where it will not be available. Note that you can return Promise.resolve() instead of new Promise(r => r()).
Attachment #8780712 -
Flags: review?(paolo.mozmail)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Reporter | ||
Comment 9•6 years ago
|
||
mozreview-review |
Comment on attachment 8780712 [details] Bug 1292566 - The "open" button in the subview for uncommon and potentially unwanted downloads should not ask for confirmation. https://reviewboard.mozilla.org/r/71352/#review71986 Thanks!
Attachment #8780712 -
Flags: review?(paolo.mozmail) → review+
Comment 10•6 years ago
|
||
Pushed by dwillcoxon@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/04efdec5c0cf The "open" button in the subview for uncommon and potentially unwanted downloads should not ask for confirmation. r=Paolo
Comment 11•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/04efdec5c0cf
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
Assignee | ||
Comment 12•6 years ago
|
||
Comment on attachment 8780712 [details] Bug 1292566 - The "open" button in the subview for uncommon and potentially unwanted downloads should not ask for confirmation. Approval Request Comment [Feature/regressing bug #]: Sliding subview for blocked download info in the downloads panel, bug 1252509 [User impact if declined]: The "open" button in the subview for blocked downloads will ask for confirmation [Describe test coverage new/current, TreeHerder]: mochitest [Risks and why]: Low risk, has automated and manual testing [String/UUID change made/needed]: None
Attachment #8780712 -
Flags: approval-mozilla-aurora?
Updated•6 years ago
|
Iteration: --- → 51.2 - Aug 29
Hi Paolo, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(paolo.mozmail)
status-firefox50:
--- → affected
Comment on attachment 8780712 [details] Bug 1292566 - The "open" button in the subview for uncommon and potentially unwanted downloads should not ask for confirmation. Fixes a regression in a new feature planned for Fx50.
Attachment #8780712 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Reporter | ||
Comment 15•6 years ago
|
||
Tested that the issue is solved on the latest Nightly on Windows 7.
Flags: needinfo?(paolo.mozmail)
Comment 16•6 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/bac8c738419e
Updated•6 years ago
|
Flags: qe-verify+
Comment 17•6 years ago
|
||
(In reply to :Paolo Amadini from comment #0) > The Downloads Panel subview for blocked downloads is designed as a > replacement and not an addition to the previous modal confirmation dialog, > so the "open" button should not ask for confirmation. The title of the bug specifies "uncommon and potentially unwanted" only. Now the "open" button doesn't ask for confirmation for any type of bad download (malicious, dangerous etc). Is this correct?
Flags: needinfo?(adw)
Assignee | ||
Comment 18•6 years ago
|
||
(In reply to Paul Silaghi, QA [:pauly] from comment #17) > The title of the bug specifies "uncommon and potentially unwanted" only. > Now the "open" button doesn't ask for confirmation for any type of bad > download (malicious, dangerous etc). Is this correct? Yes, that's my understanding, but to be safe let's ask Paolo.
Flags: needinfo?(adw) → needinfo?(paolo.mozmail)
Reporter | ||
Comment 19•6 years ago
|
||
Yeah, malware also should not ask for confirmation with the new design, because more information about the choice is already provided in the subview. I've updated the subject to match.
Flags: needinfo?(paolo.mozmail)
Summary: The "open" button in the subview for uncommon and potentially unwanted downloads should not ask for confirmation → The "open" button in the subview for temporarily blocked downloads should not ask for confirmation
Comment 20•6 years ago
|
||
Verified fixed FX 50b2, 51.0a2 (2016-09-28) Win 7 using http://testsafebrowsing.appspot.com/
Updated•6 years ago
|
Version: unspecified → 50 Branch
You need to log in
before you can comment on or make changes to this bug.
Description
•