1733492 - Downloading a file with "always ask" selected does not consistently open unknownContentType window if download panel improvements pref is enabled

Status: VERIFIED FIXED

(Firefox :: Downloads Panel, defect)

Description

[kpatenio]

Observed behaviour when browser.download.improvements_to_download_panel is true and the "always ask" content type preference is selected:

[Reproduction Steps]

1. Go to about:config and enable browser.download.improvements_to_download_panel
2. Go to about:preferences and set "Always ask" for Portable Document Format (PDF)
3. Download a PDF file (ex. https://filesamples.com/formats/pdf#google_vignette)
4. See unknownContentType window. Select any radio button option.
5. Press OK button.
6. File should download with chosen option. Wait till download is finished.
7. Download another PDF file again. unknownContentType window will not appear again.

Comment 1

[kpatenio]

Videos showing behaviour:

• Save file
• External
• Internal - Nightly

Comment 2

[Micah [:mtigley] (she/her)]

There are 2 things going on here:

• We made the decision to show the UCT dialog if the user has "Always ask" configured. See https://bugzilla.mozilla.org/show_bug.cgi?id=1719901#c4 and https://bugzilla.mozilla.org/show_bug.cgi?id=1719901#c7. Which we do seem honor the first time we download that file type and that can be observed in the video. BUT:
• Selecting one of the radio buttons seems to save the preferred action even though the checkbox for remembering the decision hasn't been checked. I believe that unless that checkbox has been selected, then it should continue to show the UCT dialog, which is the bug here.

This is probably something we want to investigate before enabling the pref in Nightly.

Comment 3

[Neil Deakin]

The preferences window (main.js) assumes that if 'alwaysAskBeforeHandling' is set for that type, then the value is 'always ask'. Changing this to 'Always Ask' from the menu sets both the preferredAction to ask and alwaysAskBeforeHandling to true.

The 'alwaysAskBeforeHandling' flag is also the one used for the checkbox in the unknown content type dialog. The 'preferredAction' however is changed when this dialog is closed to whatever was selected from the dialog. The dialog defaults to 'save' so this is what is assigned.

But since bug 1710941, when the browser_download_improvements_to_download_panel preference is enabled, the 'alwaysAskBeforeHandling' flag is ignored at https://searchfox.org/mozilla-central/rev/01adc17c9a41d9f7975de170acc78634bd743609/uriloader/exthandler/nsExternalHelperAppService.cpp#1812 so the preferred action of 'save' is used instead.

I'm not sure what the intended behaviour is here considering that bug 1710941 was recently completed.

Comment 4

[:Gijs (he/him)]

(In reply to Neil Deakin from comment #3)

The preferences window (main.js) assumes that if 'alwaysAskBeforeHandling' is set for that type, then the value is 'always ask'. Changing this to 'Always Ask' from the menu sets both the preferredAction to ask and alwaysAskBeforeHandling to true.

The 'alwaysAskBeforeHandling' flag is also the one used for the checkbox in the unknown content type dialog. The 'preferredAction' however is changed when this dialog is closed to whatever was selected from the dialog. The dialog defaults to 'save' so this is what is assigned.

But since bug 1710941, when the browser_download_improvements_to_download_panel preference is enabled, the 'alwaysAskBeforeHandling' flag is ignored at https://searchfox.org/mozilla-central/rev/01adc17c9a41d9f7975de170acc78634bd743609/uriloader/exthandler/nsExternalHelperAppService.cpp#1812 so the preferred action of 'save' is used instead.

I'm not sure what the intended behaviour is here considering that bug 1710941 was recently completed.

Micah and I talked about this a bit today, but she's busy with another project so I'm replying here. I think Micah's summary in comment #2 is accurate. We want to end up in a place where:

1. by default, mimetypes we've never seen before save to disk
2. for mimetypes where the user has made a choice they want to be asked, we show the dialog

I think bug 1710941 implemented this by, as you noted, writing to both the preferredAction and to the alwaysAskBeforeHandling bool, and then ignoring the latter. In hindsight, I think that was probably not the right solution. I think we should only rely on the alwaysAskBeforeHandling bool, as that's used in the dialog and in the rest of the code. And I think the default value for a new mimetype instance should depend on the pref, in other words, I think instead of never reading the alwaysAskBeforeHandling flag if the pref is set, we should read the flag as before - but it should default to false when the pref is set. This is on the assumption that we overwrite that false value based on info from the handlers.json store in HandlerService.js, which I believe is the case ( https://searchfox.org/mozilla-central/rev/1dab956095020c53aa66cbee8845eb1fa925b75c/uriloader/exthandler/HandlerService.js#550 ). So I think we want to change nsMimeInfoImpl to initialize to false if the "improvements" pref is set to true.

Does that make sense?

Comment 5

[Neil Deakin]

Attachment: Bug 1733492, unknown content type dialog not always appearing when always ask is set in preferences for a type, r=mtigley

This defaults the always ask setting for a type to false when the download panel improvements preference is enabled, and adjusts the external helper apps service so that it properly uses the always ask flag in all cases

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Cc'ing bigiri, because I know he's also hacking around in this area for bug 1733621, and this seems relevant.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15d91dbbc66b
unknown content type dialog not always appearing when always ask is set in preferences for a type, r=mtigley

Comment 8

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/15d91dbbc66b

Comment 9

[Simona Badau, Desktop QA]

Reproduced this on Windows 10 x64 and Firefox 94.0.1.
Verified as fixed using the latest Nightly 96.0a1 and Firefox 95 beta 3 on Windows 10 x64, Ubuntu 20.04 x64, and macOS Big Sur 11.6.