Bugzilla

(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.

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?

(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?