The "open" button in the subview for temporarily blocked downloads should not ask for confirmation

VERIFIED FIXED in Firefox 50

Status

()

defect
P2
normal
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: Paolo, Assigned: adw)

Tracking

({regression})

50 Branch
Firefox 51
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox50+ verified, firefox51 verified)

Details

(Whiteboard: [fxprivacy])

Attachments

(1 attachment)

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.
[Tracking Requested - why for this release]:
User interface regression with a new feature developed for Firefox 50 in bug 1252509.
How is this a regression?  The feature didn't exist before 50.
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!
Priority: -- → P2
Whiteboard: [fxprivacy][triage] → [fxprivacy][triage]
Assignee: nobody → adw
Status: NEW → ASSIGNED
Whiteboard: [fxprivacy][triage] → [fxprivacy]
(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.
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 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+
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
https://hg.mozilla.org/mozilla-central/rev/04efdec5c0cf
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
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?
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)
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+
Tested that the issue is solved on the latest Nightly on Windows 7.
Flags: needinfo?(paolo.mozmail)
Flags: qe-verify+
(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)
(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)
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
Verified fixed FX 50b2, 51.0a2 (2016-09-28) Win 7 using http://testsafebrowsing.appspot.com/
Status: RESOLVED → VERIFIED
Version: unspecified → 50 Branch
You need to log in before you can comment on or make changes to this bug.