1146907 - Dismissing the unblock related dialog window unblocks the download.

Status: VERIFIED FIXED

(Firefox :: Downloads Panel, defect)

Description

[Catalin Varga [QA][:VarCat]]

Environment:

FF 38
Build id: 20150323004010
OS: Win 7 x64, Ubuntu 14.04 x86, Mac Os X 10.7.5

STR:

1. Go to http://www.eicar.org/85-0-Download.html 
2. Download eicar.com 68 Bytes 
3. From the download panel Unblock eicar.com
4. Dismiss the unblock related dialog window.

Issue:
Dismissing the unblock related dialog window unblocks the download instead of keeping it blocked.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Madhava, this bug is due to bug 345067 and uncovers an issue that we won't be able to get fixed for Firefox 38.

This bug means that when a user clicks on the "unblock" button, they are shown a dialog asking them if they are sure they want to unblock the malicious download.

The default button on this dialog is "keep me safe" which keeps the download blocked. The other option on the dialog is to "unblock anyway" which allows the malicious download to be used. If the user clicks the "X" button, or hits Escape, then "unblock anyway" is chosen.

This violates user expectations since "X"/Escape is synonymous with "cancel", which in the way that the wording of this dialog is it would equate to "keep me safe". However, we wanted the default button to be "keep me safe", therefore "cancel" actually maps to "unblock anyways"

So these are our two options:
1) Do nothing, users who hit the X or Escape will have it unblocked unexpectedly.
2) Change the default button to "unblock anyways". User who hit Enter when the dialog appears will unblock the download, but Escape will cancel the action as many users have been trained to do.

What should we do here?

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

After talking with Gavin, we've got this:

The button ordering doesn't match the way that UX has requested, but the default button stays "keep me safe" and Escape/X will result in "keep me safe" behavior.

Comment 3

[:Paolo Amadini]

Comment on attachment 8582544 [details] [diff] [review]
Patch

Review of attachment 8582544 [details] [diff] [review]:
-----------------------------------------------------------------

Interesting bug!

::: browser/components/downloads/DownloadsCommon.jsm
@@ +550,5 @@
>    confirmUnblockDownload: Task.async(function* (aType, aOwnerWindow) {
>      let s = DownloadsCommon.strings;
>      let title = s.unblockHeader;
>      let buttonFlags = (Ci.nsIPrompt.BUTTON_TITLE_IS_STRING * Ci.nsIPrompt.BUTTON_POS_0) +
> +                      (Ci.nsIPrompt.BUTTON_TITLE_IS_STRING * Ci.nsIPrompt.BUTTON_POS_1 + Ci.nsIPrompt.BUTTON_POS_1_DEFAULT);

nit: you can close the parenthesis on the second line and indent the third option on the third line.

@@ +592,5 @@
>        }
>      });
>  
>      let rv = Services.prompt.confirmEx(aOwnerWindow, title, message, buttonFlags,
> +                                       okButton, cancelButton, null, null, {});

Should add a comment on why the ordering of the buttons is chosen this way.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/integration/fx-team/rev/aebc558ecfea

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8582544 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: regression found in bug 1137909
[User impact if declined]: users may accidentally unblock a malicious download
[Describe test coverage new/current, TreeHerder]: mochitest covers this codepath
[Risks and why]: none expected
[String/UUID change made/needed]: none

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/aebc558ecfea

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/154f30b91df3

Comment 8

[Catalin Varga [QA][:VarCat]]

Verified as fixed using the following environment:

FF 38.0b2
Build Id: 20150406174117
OS: Win 7 x64, Mac Os X 10.9.5, Ubuntu 14.04 x64

FF 39
Build id: 20150409004007
OS: Win 7 x64