GitLab

at 2023-04-13T16:15:53+00:00

Revert "Bug 19273: Avoid JavaScript patching of the external app helper dialog."

Bug 40701: Improve security warning when downloading a file
This reverts commit 5d003f7c5e0910c8e0c6f4e55a0ca7aa002faad1.

@@ -64,11 +64,15 @@ pref("extensions.torbutton.startup", false);
  // Security prefs:
  pref("extensions.torbutton.resize_new_windows", false);
 -pref("extensions.torbutton.launch_warning", true);
  // Browser home page:
  pref("browser.startup.homepage", "about:tor");
 +// tor-browser#40701: Add new download warning and flipping prefs to match Firefox
 +pref("browser.download.useDownloadDir", true);
 +pref("browser.download.always_ask_before_handling_new_types", false);
 +pref("browser.download.showTorWarning", true);
++
  // This pref specifies an ad-hoc "version" for various pref update hacks we need to do
  pref("extensions.torbutton.pref_fixup_version", 0);

@@ -95,6 +95,32 @@
    padding: 0.62em;
+ }
 +#downloadsPanel-mainView {
 +  /* Fix the layout to ensure the #downloadsWarningDescription is given enough
 +   * vertical space. For tor-browser#40701.
 +   * TODO: May no longer be necessary after esr 115 due to bugzilla bug 1816455.
 +   */
 +  display: flex;
 +  flex-direction: column;
 +}
++
 +#downloadsWarning p {
 +  padding-inline: 8px;
 +  margin-block-start: 8px;
 +  margin-block-end: 0;
 +  line-height: 1.4;
 +}
++
 +#downloadsWarningHeaderTitle {
 +  font-weight: bold;
 +}
++
 +#downloadsWarningDescription {
 +  /* Make sure we wrap the text rather than request the default max-content
 +   * width from the parent XUL -moz-box. */
 +  width: 0;
 +}
++
  #downloadsHistory,
  #downloadsFooterButtons {
    margin: 0;

@@ -31,6 +31,10 @@
  "use strict";
 +const PREF_SHOW_DOWNLOAD_WARNING = "browser.download.showTorWarning";
++
 +const TAILS_URI = "https://tails.boum.org/";
++
  var { XPCOMUtils } = ChromeUtils.import(
    "resource://gre/modules/XPCOMUtils.jsm"
  );
@@ -93,6 +97,13 @@ var DownloadsPanel = {
     */
    _state: 0,
 +  /**
 +   * State of tor's download warning. It should only be initialized once (text assigned,
 +   * button commands assigned). This tracks if that has been performed and prevents
 +   * repeats.
 +   */
 +  _torWarningInitialized: false,
++
    /** The panel is not linked to downloads data yet. */
    get kStateUninitialized() {
      return 0;
@@ -132,6 +143,15 @@ var DownloadsPanel = {
        );
+     }
 +    let showDownloadWarning = Services.prefs.getBoolPref(
 +      PREF_SHOW_DOWNLOAD_WARNING
 +    );
 +    if (!showDownloadWarning) {
 +      document.getElementById("downloadsWarning").hidden = true;
 +    } else {
 +      document.getElementById("downloadsWarning").hidden = false;
 +    }
++
      if (this._state != this.kStateUninitialized) {
        DownloadsCommon.log("DownloadsPanel is already initialized.");
        return;
@@ -155,6 +175,44 @@ var DownloadsPanel = {
        DownloadsSummary
      );
 +    if (this._torWarningInitialized == 0) {
 +      document.getElementById(
 +        "downloadsWarningHeaderTitle"
 +      ).textContent = this._getString("torbutton.download.warning.title");
 +      let tailsBrandName = this._getString(
 +        "torbutton.download.warning.tails_brand_name"
 +      );
++
 +      let warningDescriptionText = this._getString(
 +        "torbutton.download.warning.description"
 +      );
 +      let [head, rest] = warningDescriptionText.split("%S");
 +      const tailsLink = document.createElement("a");
 +      tailsLink.setAttribute("href", TAILS_URI);
 +      tailsLink.textContent = tailsBrandName.trim();
 +      tailsLink.addEventListener("click", event => {
 +        event.preventDefault();
 +        this.hidePanel();
 +        openWebLinkIn(TAILS_URI, "tab");
 +      });
++
 +      let downloadsWarningDescription = document.getElementById(
 +        "downloadsWarningDescription"
 +      );
 +      downloadsWarningDescription.append(head, tailsLink, rest);
++
 +      let dismissBtn = document.getElementById(
 +        "downloadWarningDismiss"
 +      );
 +      dismissBtn.textContent = this._getString("torbutton.download.warning.dismiss");
 +      dismissBtn.addEventListener("click", event => {
 +        Services.prefs.setBoolPref(PREF_SHOW_DOWNLOAD_WARNING, false);
 +        document.getElementById("downloadsWarning").hidden = true;
 +        this._focusPanel(true);
 +      });
 +      this._torWarningInitialized = 1;
 +    }
++
      DownloadsCommon.log(
        "DownloadsView attached - the panel for this window",
        "should now see download items come in."
@@ -530,16 +588,21 @@ var DownloadsPanel = {
    /**
     * Move focus to the main element in the downloads panel, unless another
     * element in the panel is already focused.
 +   *
 +   * @param {bool} [forceFocus=false] - Whether to force move the focus.
     */
 -  _focusPanel() {
 -    // We may be invoked while the panel is still waiting to be shown.
 -    if (this._state != this.kStateShown) {
 -      return;
 -    }
 +  _focusPanel(forceFocus=false) {
 +    if (!forceFocus) {
 +      // We may be invoked while the panel is still waiting to be shown.
 +      if (this._state != this.kStateShown) {
 +        return;
 +      }
 -    if (document.activeElement && this.panel.contains(document.activeElement)) {
 -      return;
 +      if (document.activeElement && this.panel.contains(document.activeElement)) {
 +        return;
 +      }
+     }
++
      let focusOptions = { preventFocusRing: !!this._preventFocusRing };
      if (DownloadsView.richListBox.itemCount > 0) {
        DownloadsView.richListBox.selectedIndex = 0;
@@ -658,6 +721,30 @@ var DownloadsPanel = {
+       }
      }, 0);
    },
++
 +  /**
 +   * Get a string from the properties bundle.
 +   *
 +   * @param {string} name - The string name.
 +   *
 +   * @return {string} The string.
 +   */
 +  _getString(name) {
 +    if (!this._stringBundle) {
 +      this._stringBundle = Services.strings.createBundle(
 +        "chrome://torbutton/locale/torbutton.properties"
 +      );
 +    }
 +    try {
 +      return this._stringBundle.GetStringFromName(name);
 +    } catch {}
 +    if (!this._fallbackStringBundle) {
 +      this._fallbackStringBundle = Services.strings.createBundle(
 +        "resource://torbutton/locale/en-US/torbutton.properties"
 +      );
 +    }
 +    return this._fallbackStringBundle.GetStringFromName(name);
 +  },
  };
  XPCOMUtils.defineConstant(this, "DownloadsPanel", DownloadsPanel);

@@ -124,6 +124,21 @@
                    disablekeynav="true">
      <panelview id="downloadsPanel-mainView">
 +      <vbox id="downloadsWarning">
 +        <vbox role="alert"
 +              aria-labelledby="downloadsWarningHeaderTitle"
 +              aria-describedby="downloadsWarningDescription">
 +          <html:p id="downloadsWarningHeaderTitle"></html:p>
 +          <html:p id="downloadsWarningDescription">
 +          </html:p>
++
 +          <html:div class="panel-footer">
 +            <html:button id="downloadWarningDismiss">
 +            </html:button>
 +          </html:div>
 +        </vbox>
 +        <toolbarseparator />
 +      </vbox>
        <vbox class="panel-view-body-unscrollable">
          <richlistbox id="downloadsListBox"
                       data-l10n-id="downloads-panel-items"

@@ -1324,6 +1324,7 @@ panelview .toolbarbutton-1 {
  #downloadsFooterButtons > toolbarseparator,
  .cui-widget-panelview menuseparator,
  .cui-widget-panel toolbarseparator,
 +#downloadsWarning toolbarseparator,
  #securityLevel-panel toolbarseparator {
    appearance: none;
    min-height: 0;

Richard Pospesel pushed to branch tor-browser-102.10.0esr-12.5-1 at The Tor Project / Applications / Tor Browser

Commits:

12 changed files:

Changes:

...	...	@@ -258,6 +258,7 @@
258	258	}
259	259
260	260	/* Toolbarseparator */
	261	+#downloadsWarning toolbarseparator,
261	262	#downloadsFooterButtons > toolbarseparator {
262	263	margin-inline: 0;
263	264	}

@@ -32,7 +32,7 @@ torbutton.circuit_display.click_to_copy = Click to Copy
  torbutton.circuit_display.copied = Copied!
  # end
 -# External app blocker
 +# External app blocker strings used up to 12.0 - TODO: remove when 12.5 becomes stable:
  torbutton.popup.external.title = Download an external file type?
  torbutton.popup.external.app = Tor Browser cannot display this file. You will need to open it with another application.\n\n
  torbutton.popup.external.note = Some types of files can cause applications to connect to the Internet without using Tor.\n\n
@@ -41,6 +41,14 @@ torbutton.popup.launch = Download file
  torbutton.popup.cancel = Cancel
  torbutton.popup.dontask = Automatically download files from now on
 +# Download pane warning
 +torbutton.download.warning.title = Be careful opening downloads
 +# %S will be a link to the Tails operating system website. With the content given by torbutton.download.warning.tails_brand_name
 +torbutton.download.warning.description = Some files may connect to the internet when opened without using Tor. To be safe, open the files while offline or use a portable operating system like %S.
 +# Locale name for Tails operating system.
 +torbutton.download.warning.tails_brand_name = Tails
 +torbutton.download.warning.dismiss = Got it
++
  # .Onion Page Info prompt.
  pageInfo_OnionEncryptionWithBitsAndProtocol=Connection Encrypted (Onion Service, %1$S, %2$S bit keys, %3$S)
  pageInfo_OnionEncryption=Connection Encrypted (Onion Service)

 -// Bug 1506 Android P1/TBB P5: This code provides users with notification
 -// in the event of external app launch. We want it to exist in the desktop
 -// port, but it is probably useless for Android.
+-
 -/*************************************************************************
 - * External App Handler.
 - * Handles displaying confirmation dialogs for external apps and protocols
 - * due to Firefox Bug https://bugzilla.mozilla.org/show_bug.cgi?id=440892
 - *
 - * An instance of this module is created each time the browser starts to
 - * download a file and when an external application may be invoked to
 - * handle an URL (e.g., when the user clicks on a mailto: URL).
 - *************************************************************************/
+-
 -const { XPCOMUtils } = ChromeUtils.import(
 -  "resource://gre/modules/XPCOMUtils.jsm"
 -);
 -const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 -const { PromptUtils } = ChromeUtils.import(
 -  "resource://gre/modules/SharedPromptUtils.jsm"
 -);
+-
 -XPCOMUtils.defineLazyModuleGetters(this, {
 -  ComponentUtils: "resource://gre/modules/ComponentUtils.jsm",
 -});
+-
 -let { torbutton_get_property_string } = ChromeUtils.import(
 -  "resource://torbutton/modules/utils.js"
 -);
+-
 -// Module specific constants
 -const kMODULE_NAME = "Torbutton External App Handler";
 -const kCONTRACT_ID = "@torproject.org/torbutton-extAppBlocker;1";
 -const kMODULE_CID = Components.ID("3da0269f-fc29-4e9e-a678-c3b1cafcf13f");
+-
 -const kInterfaces = [Ci.nsIObserver, Ci.nsIClassInfo];
+-
 -function ExternalAppBlocker() {
 -  this.logger = Cc["@torproject.org/torbutton-logger;1"].getService(
 -    Ci.nsISupports
 -  ).wrappedJSObject;
 -  this.logger.log(3, "Component Load 0: New ExternalAppBlocker.");
 -}
+-
 -ExternalAppBlocker.prototype = {
 -  _helperAppLauncher: undefined,
+-
 -  QueryInterface: ChromeUtils.generateQI([
 -    Ci.nsIObserver,
 -    Ci.nsIHelperAppWarningDialog,
 -  ]),
+-
 -  // make this an nsIClassInfo object
 -  flags: Ci.nsIClassInfo.DOM_OBJECT,
 -  classDescription: kMODULE_NAME,
 -  contractID: kCONTRACT_ID,
 -  classID: kMODULE_CID,
+-
 -  // method of nsIClassInfo
 -  getInterfaces(count) {
 -    count.value = kInterfaces.length;
 -    return kInterfaces;
 -  },
+-
 -  // method of nsIClassInfo
 -  getHelperForLanguage(count) {
 -    return null;
 -  },
+-
 -  // method of nsIHelperAppWarningDialog
 -  maybeShow(aLauncher, aWindowContext) {
 -    // Hold a reference to the object that called this component. This is
 -    // important not just because we need to later invoke the
 -    // continueRequest() or cancelRequest() callback on aLauncher, but also
 -    // so that the launcher object (which is a reference counted object) is
 -    // not released too soon.
 -    this._helperAppLauncher = aLauncher;
+-
 -    if (!Services.prefs.getBoolPref("extensions.torbutton.launch_warning")) {
 -      this._helperAppLauncher.continueRequest();
 -      return;
 -    }
+-
 -    this._showPrompt(aWindowContext);
 -  },
+-
 -  /*
 -   * The _showPrompt() implementation uses some XUL and JS that is part of the
 -   * browser's confirmEx() implementation. Specifically, _showPrompt() depends
 -   * on chrome://global/content/commonDialog.xhtml as well as some of the code
 -   * in resource://gre/modules/SharedPromptUtils.jsm.
 -   */
 -  _showPrompt(aWindowContext) {
 -    let parentWin;
 -    try {
 -      parentWin = aWindowContext.getInterface(Ci.nsIDOMWindow);
 -    } catch (e) {
 -      parentWin = Services.wm.getMostRecentWindow("navigator:browser");
 -    }
+-
 -    let title = torbutton_get_property_string("torbutton.popup.external.title");
 -    let app = torbutton_get_property_string("torbutton.popup.external.app");
 -    let note = torbutton_get_property_string("torbutton.popup.external.note");
 -    let suggest = torbutton_get_property_string(
 -      "torbutton.popup.external.suggest"
 -    );
 -    let launch = torbutton_get_property_string("torbutton.popup.launch");
 -    let cancel = torbutton_get_property_string("torbutton.popup.cancel");
 -    let dontask = torbutton_get_property_string("torbutton.popup.dontask");
+-
 -    let args = {
 -      promptType: "confirmEx",
 -      title,
 -      text: app + note + suggest + " ",
 -      checkLabel: dontask,
 -      checked: false,
 -      ok: false,
 -      button0Label: launch,
 -      button1Label: cancel,
 -      defaultButtonNum: 1, // Cancel
 -      buttonNumClicked: 1, // Cancel
 -      enableDelay: true,
 -    };
+-
 -    let propBag = PromptUtils.objectToPropBag(args);
 -    let uri = "chrome://global/content/commonDialog.xhtml";
 -    let promptWin = Services.ww.openWindow(
 -      parentWin,
 -      uri,
 -      "_blank",
 -      "centerscreen,chrome,titlebar",
 -      propBag
 -    );
 -    promptWin.addEventListener("load", aEvent => {
 -      promptWin.addEventListener("unload", aEvent => {
 -        PromptUtils.propBagToObject(propBag, args);
+-
 -        if (0 == args.buttonNumClicked) {
 -          // Save the checkbox value and tell the browser's external helper app
 -          // module about the user's choice.
 -          if (args.checked) {
 -            Services.prefs.setBoolPref(
 -              "extensions.torbutton.launch_warning",
 -              false
 -            );
 -          }
+-
 -          this._helperAppLauncher.continueRequest();
 -        } else {
 -          this._helperAppLauncher.cancelRequest(Cr.NS_BINDING_ABORTED);
 -        }
 -      });
 -    });
 -  },
 -};
+-
 -// Assign factory to global object.
 -const NSGetFactory = XPCOMUtils.generateNSGetFactory
 -  ? XPCOMUtils.generateNSGetFactory([ExternalAppBlocker])
 -  : ComponentUtils.generateNSGetFactory([ExternalAppBlocker]);

@@ -34,9 +34,6 @@ torbutton.jar:
  % component {f605ec27-d867-44b5-ad97-2a29276642c3} %components/dragDropFilter.js
  % contract @torproject.org/torbutton-dragDropFilter;1 {f605ec27-d867-44b5-ad97-2a29276642c3}
 -% component {3da0269f-fc29-4e9e-a678-c3b1cafcf13f} %components/external-app-blocker.js
 -% contract @torproject.org/torbutton-extAppBlocker;1 {3da0269f-fc29-4e9e-a678-c3b1cafcf13f}
+-
  % component {06322def-6fde-4c06-aef6-47ae8e799629} %components/startup-observer.js
  % contract @torproject.org/startup-observer;1 {06322def-6fde-4c06-aef6-47ae8e799629}

@@ -144,9 +144,6 @@ static const char NEVER_ASK_FOR_SAVE_TO_DISK_PREF[] =
  static const char NEVER_ASK_FOR_OPEN_FILE_PREF[] =
      "browser.helperApps.neverAsk.openFile";
 -static const char WARNING_DIALOG_CONTRACT_ID[] =
 -    "@torproject.org/torbutton-extAppBlocker;1";
+-
  StaticRefPtr<nsIFile> sFallbackDownloadDir;
  // Helper functions for Content-Disposition headers
@@ -398,22 +395,6 @@ nsresult GenerateRandomName(nsACString& result) {
    return NS_OK;
+ }
 -static already_AddRefed<nsIInterfaceRequestor> GetDialogParentAux(
 -    BrowsingContext* aBrowsingContext, nsIInterfaceRequestor* aWindowContext) {
 -  nsCOMPtr<nsIInterfaceRequestor> dialogParent = aWindowContext;
+-
 -  if (!dialogParent && aBrowsingContext) {
 -    dialogParent = do_QueryInterface(aBrowsingContext->GetDOMWindow());
 -  }
 -  if (!dialogParent && aBrowsingContext && XRE_IsParentProcess()) {
 -    RefPtr<Element> element = aBrowsingContext->Top()->GetEmbedderElement();
 -    if (element) {
 -      dialogParent = do_QueryInterface(element->OwnerDoc()->GetWindow());
 -    }
 -  }
 -  return dialogParent.forget();
 -}
+-
  /**
   * Structure for storing extension->type mappings.
   * @see defaultMimeEntries
@@ -611,96 +592,6 @@ static const char* descriptionOverwriteExtensions[] = {
      "avif", "jxl", "pdf", "svg", "webp", "xml",
  };
 -//////////////////////////////////////////////////////////////////////////////////////////////////////
 -// begin nsExternalLoadURIHandler class definition and implementation
 -//////////////////////////////////////////////////////////////////////////////////////////////////////
 -class nsExternalLoadURIHandler final : public nsIHelperAppWarningLauncher {
 - public:
 -  NS_DECL_THREADSAFE_ISUPPORTS
 -  NS_DECL_NSIHELPERAPPWARNINGLAUNCHER
+-
 -  nsExternalLoadURIHandler(nsIHandlerInfo* aHandlerInfo, nsIURI* aURI,
 -                           nsIPrincipal* aTriggeringPrincipal,
 -                           BrowsingContext* aBrowsingContext,
 -                           bool aTriggeredExternally);
+-
 - protected:
 -  ~nsExternalLoadURIHandler();
+-
 -  nsCOMPtr<nsIHandlerInfo> mHandlerInfo;
 -  nsCOMPtr<nsIURI> mURI;
 -  nsCOMPtr<nsIPrincipal> mTriggeringPrincipal;
 -  RefPtr<BrowsingContext> mBrowsingContext;
 -  bool mTriggeredExternally;
 -  nsCOMPtr<nsIHelperAppWarningDialog> mWarningDialog;
 -};
+-
 -NS_IMPL_ADDREF(nsExternalLoadURIHandler)
 -NS_IMPL_RELEASE(nsExternalLoadURIHandler)
+-
 -NS_INTERFACE_MAP_BEGIN(nsExternalLoadURIHandler)
 -  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIHelperAppWarningLauncher)
 -  NS_INTERFACE_MAP_ENTRY(nsIHelperAppWarningLauncher)
 -NS_INTERFACE_MAP_END
+-
 -nsExternalLoadURIHandler::nsExternalLoadURIHandler(
 -    nsIHandlerInfo* aHandlerInfo, nsIURI* aURI,
 -    nsIPrincipal* aTriggeringPrincipal, BrowsingContext* aBrowsingContext,
 -    bool aTriggeredExternally)
 -    : mHandlerInfo(aHandlerInfo),
 -      mURI(aURI),
 -      mTriggeringPrincipal(aTriggeringPrincipal),
 -      mBrowsingContext(aBrowsingContext),
 -      mTriggeredExternally(aTriggeredExternally)
+-
 -{
 -  nsresult rv = NS_OK;
 -  mWarningDialog = do_CreateInstance(WARNING_DIALOG_CONTRACT_ID, &rv);
 -  if (NS_SUCCEEDED(rv) && mWarningDialog) {
 -    // This will create a reference cycle (the dialog holds a reference to us
 -    // as nsIHelperAppWarningLauncher), which will be broken in ContinueRequest
 -    // or CancelRequest.
 -    nsCOMPtr<nsIInterfaceRequestor> dialogParent =
 -        GetDialogParentAux(aBrowsingContext, nullptr);
 -    rv = mWarningDialog->MaybeShow(this, dialogParent);
 -  }
+-
 -  if (NS_FAILED(rv)) {
 -    // If for some reason we could not open the download warning prompt,
 -    // continue with the request.
 -    ContinueRequest();
 -  }
 -}
+-
 -nsExternalLoadURIHandler::~nsExternalLoadURIHandler() {}
+-
 -NS_IMETHODIMP nsExternalLoadURIHandler::ContinueRequest() {
 -  MOZ_ASSERT(mURI);
 -  MOZ_ASSERT(mHandlerInfo);
+-
 -  // Break our reference cycle with the download warning dialog (set up in
 -  // LoadURI).
 -  mWarningDialog = nullptr;
+-
 -  nsresult rv = NS_OK;
 -  nsCOMPtr<nsIContentDispatchChooser> chooser =
 -      do_CreateInstance("@mozilla.org/content-dispatch-chooser;1", &rv);
 -  NS_ENSURE_SUCCESS(rv, rv);
+-
 -  return chooser->HandleURI(mHandlerInfo, mURI, mTriggeringPrincipal,
 -                            mBrowsingContext, mTriggeredExternally);
 -}
+-
 -NS_IMETHODIMP nsExternalLoadURIHandler::CancelRequest(nsresult aReason) {
 -  NS_ENSURE_ARG(NS_FAILED(aReason));
+-
 -  // Break our reference cycle with the download warning dialog (set up in
 -  // LoadURI).
 -  mWarningDialog = nullptr;
+-
 -  return NS_OK;
 -}
+-
  static StaticRefPtr<nsExternalHelperAppService> sExtHelperAppSvcSingleton;
  /**
@@ -723,9 +614,6 @@ nsExternalHelperAppService::GetSingleton() {
    return do_AddRef(sExtHelperAppSvcSingleton);
+ }
 -//////////////////////////////////////////////////////////////////////////////////////////////////////
 -// nsExternalHelperAppService definition and implementation
 -//////////////////////////////////////////////////////////////////////////////////////////////////////
  NS_IMPL_ISUPPORTS(nsExternalHelperAppService, nsIExternalHelperAppService,
                    nsPIExternalAppLauncher, nsIExternalProtocolService,
                    nsIMIMEService, nsIObserver, nsISupportsWeakReference)
@@ -1238,15 +1126,14 @@ nsExternalHelperAppService::LoadURI(nsIURI* aURI,
    rv = GetProtocolHandlerInfo(scheme, getter_AddRefs(handler));
    NS_ENSURE_SUCCESS(rv, rv);
 -  RefPtr<nsExternalLoadURIHandler> h = new nsExternalLoadURIHandler(
 +  nsCOMPtr<nsIContentDispatchChooser> chooser =
 +      do_CreateInstance("@mozilla.org/content-dispatch-chooser;1", &rv);
 +  NS_ENSURE_SUCCESS(rv, rv);
++
 +  return chooser->HandleURI(
        handler, escapedURI,
        aRedirectPrincipal ? aRedirectPrincipal : aTriggeringPrincipal,
        aBrowsingContext, aTriggeredExternally);
 -  if (!h) {
 -    return NS_ERROR_OUT_OF_MEMORY;
 -  }
+-
 -  return NS_OK;
+ }
  //////////////////////////////////////////////////////////////////////////////////////////////////////
@@ -1391,7 +1278,6 @@ NS_INTERFACE_MAP_BEGIN(nsExternalAppHandler)
    NS_INTERFACE_MAP_ENTRY(nsIStreamListener)
    NS_INTERFACE_MAP_ENTRY(nsIRequestObserver)
    NS_INTERFACE_MAP_ENTRY(nsIHelperAppLauncher)
 -  NS_INTERFACE_MAP_ENTRY(nsIHelperAppWarningLauncher)
    NS_INTERFACE_MAP_ENTRY(nsICancelable)
    NS_INTERFACE_MAP_ENTRY(nsIBackgroundFileSaverObserver)
    NS_INTERFACE_MAP_ENTRY(nsINamed)
@@ -1647,7 +1533,18 @@ void nsExternalAppHandler::MaybeApplyDecodingForExtension(
  already_AddRefed<nsIInterfaceRequestor>
  nsExternalAppHandler::GetDialogParent() {
 -  return GetDialogParentAux(mBrowsingContext, mWindowContext);
 +  nsCOMPtr<nsIInterfaceRequestor> dialogParent = mWindowContext;
++
 +  if (!dialogParent && mBrowsingContext) {
 +    dialogParent = do_QueryInterface(mBrowsingContext->GetDOMWindow());
 +  }
 +  if (!dialogParent && mBrowsingContext && XRE_IsParentProcess()) {
 +    RefPtr<Element> element = mBrowsingContext->Top()->GetEmbedderElement();
 +    if (element) {
 +      dialogParent = do_QueryInterface(element->OwnerDoc()->GetWindow());
 +    }
 +  }
 +  return dialogParent.forget();
+ }
  NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest* request) {
@@ -1795,34 +1692,6 @@ NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest* request) {
      loadInfo->SetForceAllowDataURI(true);
+   }
 -  mWarningDialog = do_CreateInstance(WARNING_DIALOG_CONTRACT_ID, &rv);
 -  if (NS_SUCCEEDED(rv) && mWarningDialog) {
 -    // This will create a reference cycle (the dialog holds a reference to us
 -    // as nsIHelperAppWarningLauncher), which will be broken in ContinueRequest
 -    // or CancelRequest.
 -    nsCOMPtr<nsIInterfaceRequestor> dialogParent = GetDialogParent();
 -    rv = mWarningDialog->MaybeShow(this, dialogParent);
 -  }
+-
 -  if (NS_FAILED(rv)) {
 -    // If for some reason we could not open the download warning prompt,
 -    // continue with the request.
 -    ContinueRequest();
 -  }
+-
 -  return NS_OK;
 -}
+-
 -NS_IMETHODIMP nsExternalAppHandler::ContinueRequest() {
 -  nsAutoCString MIMEType;
 -  if (mMimeInfo) {
 -    mMimeInfo->GetMIMEType(MIMEType);
 -  }
+-
 -  // Break our reference cycle with the download warning dialog (set up in
 -  // OnStartRequest).
 -  mWarningDialog = nullptr;
+-
    // now that the temp file is set up, find out if we need to invoke a dialog
    // asking the user what they want us to do with this content...
@@ -1968,7 +1837,7 @@ NS_IMETHODIMP nsExternalAppHandler::ContinueRequest() {
        GetTargetFile(getter_AddRefs(fileToTest));
        if (fileToTest) {
          bool isExecutable;
 -        nsresult rv = fileToTest->IsExecutable(&isExecutable);
 +        rv = fileToTest->IsExecutable(&isExecutable);
          if (NS_FAILED(rv) || mTempFileIsExecutable ||
              isExecutable) {  // checking NS_FAILED, because paranoia is good
            alwaysAsk = true;
@@ -1984,7 +1853,20 @@ NS_IMETHODIMP nsExternalAppHandler::ContinueRequest() {
+   }
  #endif
 -  nsresult rv = NS_OK;
 +  nsAutoCString actionTelem;
 +  if (alwaysAsk) {
 +    actionTelem.AssignLiteral("ask");
 +  } else if (shouldAutomaticallyHandleInternally) {
 +    actionTelem.AssignLiteral("internal");
 +  } else if (action == nsIMIMEInfo::useHelperApp ||
 +             action == nsIMIMEInfo::useSystemDefault) {
 +    actionTelem.AssignLiteral("external");
 +  } else {
 +    actionTelem.AssignLiteral("save");
 +  }
++
 +  RecordDownloadTelemetry(aChannel, actionTelem.get());
++
    if (alwaysAsk) {
      // Display the dialog
      mDialog = do_CreateInstance(NS_HELPERAPPLAUNCHERDLG_CONTRACTID, &rv);
@@ -2017,14 +1899,6 @@ NS_IMETHODIMP nsExternalAppHandler::ContinueRequest() {
    return NS_OK;
+ }
 -NS_IMETHODIMP nsExternalAppHandler::CancelRequest(nsresult aReason) {
 -  // Break our reference cycle with the download warning dialog (set up in
 -  // OnStartRequest).
 -  mWarningDialog = nullptr;
+-
 -  return Cancel(aReason);
 -}
+-
  void nsExternalAppHandler::RecordDownloadTelemetry(nsIChannel* aChannel,
                                                     const char* aAction) {
    // Telemetry for helper app dialog
@@ -2888,7 +2762,7 @@ NS_IMETHODIMP nsExternalAppHandler::Cancel(nsresult aReason) {
+   }
    // Break our reference cycle with the helper app dialog (set up in
 -  // ContinueRequest)
 +  // OnStartRequest)
    mDialog = nullptr;
    mDialogShowing = false;

@@ -261,7 +261,6 @@ class nsExternalHelperAppService : public nsIExternalHelperAppService,
   */
  class nsExternalAppHandler final : public nsIStreamListener,
                                     public nsIHelperAppLauncher,
 -                                   public nsIHelperAppWarningLauncher,
                                     public nsIBackgroundFileSaverObserver,
                                     public nsINamed {
   public:
@@ -269,7 +268,6 @@ class nsExternalAppHandler final : public nsIStreamListener,
    NS_DECL_NSISTREAMLISTENER
    NS_DECL_NSIREQUESTOBSERVER
    NS_DECL_NSIHELPERAPPLAUNCHER
 -  NS_DECL_NSIHELPERAPPWARNINGLAUNCHER
    NS_DECL_NSICANCELABLE
    NS_DECL_NSIBACKGROUNDFILESAVEROBSERVER
    NS_DECL_NSINAMED
@@ -546,7 +544,6 @@ class nsExternalAppHandler final : public nsIStreamListener,
    nsCOMPtr<nsITransfer> mTransfer;
    nsCOMPtr<nsIHelperAppLauncherDialog> mDialog;
 -  nsCOMPtr<nsIHelperAppWarningDialog> mWarningDialog;
    /**

@@ -189,50 +189,3 @@ interface nsIHelperAppLauncher : nsICancelable
     */
    readonly attribute uint64_t browsingContextId;
  };
+-
 -/**
 -  * nsIHelperAppWarningLauncher is implemented by two classes:
 -  *   nsExternalLoadURIHandler
 -  *   nsExternalAppHandler
 -  */
 -[scriptable, uuid(cffd508b-4aaf-43ad-99c6-671d35cbc558)]
 -interface nsIHelperAppWarningLauncher : nsISupports
 -{
 -  /**
 -   * Callback invoked by the external app warning dialog to continue the
 -   * request.
 -   * NOTE: This will release the reference to the nsIHelperAppWarningDialog.
 -   */
 -  void continueRequest();
+-
 -  /**
 -   * Callback invoked by the external app warning dialog to cancel the request.
 -   * NOTE: This will release the reference to the nsIHelperAppWarningDialog.
 -   *
 -   * @param aReason
 -   *        Pass a failure code to indicate the reason why this operation is
 -   *        being canceled. It is an error to pass a success code.
 -   */
 -  void cancelRequest(in nsresult aReason);
 -};
+-
 -/**
 - * nsIHelperAppWarningDialog is implemented by Torbutton's external app
 - * blocker (src/components/external-app-blocker.js).
 - */
 -[scriptable, uuid(f4899a3f-0df3-42cc-9db8-bdf599e5a208)]
 -interface nsIHelperAppWarningDialog : nsISupports
 -{
 -  /**
 -   * Possibly show a launch warning dialog (it will not be shown if the user
 -   * has chosen to not see the warning again).
 -   *
 -   * @param aLauncher
 -   *        A nsIHelperAppWarningLauncher to be invoked after the user confirms
 -   *        or cancels the download.
 -   * @param aWindowContext
 -   *        The window associated with the download.
 -   */
 -  void maybeShow(in nsIHelperAppWarningLauncher aLauncher,
 -                 in nsISupports aWindowContext);
 -};