1670651 - browser.helperApps.neverAsk.saveToDisk is ignored (still get prompted) when downloading files for which we offer to "open in firefox"

Status: RESOLVED WORKSFORME

(Firefox :: File Handling, defect, P1)

Description

[Cristian Nicolae (:xti)]

Attachment: Firefox serenity configurationtxt.txt

Prerequisites:
1. Latest Firefox x64 installed (current version is 81.0.1)
2. A selenium project using latest geckodriver (current version 0.27.0)
3. The following geckodriver preferences set in the project (on my case, set on serenity.properties):

• browser.helperApps.neverAsk.saveToDisk=application/xml;
• browser.helperApps.alwaysAsk.force=false;

4. A website where an XML file can be downloaded (file MIME-TYpe is application/xml)

Steps to reproduce:

1. On the selenium automated framework, try to download an XML file, without displaying the Firefox Download popup

Actual result:
With the latest Firefox and geckodriver versions, the Firefox popup is always displayed.

Expected result:
The XML file should be directly downloaded to the default or specified downloaded directory.

The response I am getting when I click on the Save button is:

HTTP/1.1 200 OK
Date: Mon, 12 Oct 2020 12:09:10 GMT
Content-Length: 8170
Content-Type: application/xml; charset=UTF-8
Content-Disposition: attachment; filename=signed_test.xml

NOTE:
My framework is based on serenity v.2.3.5
My complete Firefox configuration on serenity.properties can be found attached to this ticket.

Comment 1

[Cristian Nicolae (:xti)]

I just updated my Firefox x64 to 81.0.2 and still I have the same issue.

In addition, I downgraded my serenity version to an older one (2.2.10) and nothing changed.

Comment 2

[Cristian Nicolae (:xti)]

I re-tested with the previous version of Firefox 80.0.1 and this issue is not reproducible.

Can you let me know please if indeed it is a bug or there is a preference update from v.81+?

Thank you!

Comment 3

[Mark Banner (:standard8)]

Please don't set random flags and assign random people, these are mainly for our purposes. Setting the severity/priority will also skip triage which will make it less likely your bug will be looked at.

Does the issue happen if you try it manually without Selenium? If it is not reproducible manually, then this is more likely geckodriver related than specific to Firefox.

Comment 4

[Cristian Nicolae (:xti)]

Thank you Mark for your feedback!

I tried to reproduce manually on Firefox v.81.0.2 and I have the same behavior.

As an additional information, the same geckodriver version (0.27) was used on the previous Firefox version (v.80) and everything was fine.

Comment 5

[:Gijs (he/him)]

This looks like it regressed with bug 1639067.

Adam, can you take a look?

(marking 81/82 wontfix because it's too late for fixes to make it to those branches as 82's release candidates builds are already being created)

Reporter: I think you can workaround for now by setting browser.download.viewableInternally.enabledTypes to the empty string (or at least removing the types you care about).

Comment 6

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

It looks like where the pref is checked (I think this is the only place that checks browser.helperApps.neverAsk.saveToDisk), is only hit if we don't have a stored handler for the datatype, and with bug 1639067 I added application/xml to the datastore by default.

What I'm not clear on is why alwaysAsk is true at this point, when it is retrieved from mMimeInfo. The stored handler has alwaysAsk false. I'm not sure what other logic would be more correct here.

I don't see browser.helperApps.alwaysAsk.force being used anywhere.

Clearing browser.download.viewableInternally.enabledTypes should work around this for the time being, though I haven't had a chance to test it.

Comment 7

[:Gijs (he/him)]

(In reply to Adam Gashlin (he/him) [:agashlin] from comment #6)

It looks like where the pref is checked (I think this is the only place that checks browser.helperApps.neverAsk.saveToDisk), is only hit if we don't have a stored handler for the datatype, and with bug 1639067 I added application/xml to the datastore by default.

Ah, makes sense.

What I'm not clear on is why alwaysAsk is true at this point, when it is retrieved from mMimeInfo. The stored handler has alwaysAsk false. I'm not sure what other logic would be more correct here.

The headers in comment #0 show Content-Disposition: attachment, which forces alwaysAsk if the default action is not saveToDisk.

Clearing browser.download.viewableInternally.enabledTypes should work around this for the time being, though I haven't had a chance to test it.

I doublechecked and it worked. :-)

So I guess to fix this we would need to move where we check these legacy prefs...

Comment 8

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #7)

So I guess to fix this we would need to move where we check these legacy prefs...

Or, alternatively, we could build a one-time migration to just store them into the handler service and call it a day - the comments in the code suggest these prefs predate mimetypes.rdf (RIP) which was replaced with the handler service's handlers.json...

Comment 9

[StephenSoftware]

Until version 80 everything worked fine, from 81 a new option appeared: open with firefox, and it is not possible to disable it, with the result that files with mime type application/xml are not saved when the browser is driven by Selenium Geckodriver in console app written in C#.
Thanks in advance for the new version that solves this problem.

Comment 10

[:Gijs (he/him)]

(In reply to StephenSoftware from comment #9)

Until version 80 everything worked fine, from 81 a new option appeared: open with firefox, and it is not possible to disable it, with the result that files with mime type application/xml are not saved when the browser is driven by Selenium Geckodriver in console app written in C#.
Thanks in advance for the new version that solves this problem.

As I said in comment #5, you can work around it for now by setting an additional Firefox preference in your config file.

Comment 11

[StephenSoftware]

Thanks to :Gijs (he/him) for the advice...

For all those who have the same problem below the workaround in C #.

            // WORKAROUND
            driver.Url = "about:config";
            driver.Manage().Window.Maximize();
            driver.Manage().Timeouts().ImplicitWait = TimeSpan.FromSeconds(10);
            IWebElement warningButton = driver.FindElement(By.Id("warningButton"));
            warningButton.Click();
            IWebElement aboutconfigsearch = driver.FindElement(By.Id("about-config-search"));
            aboutconfigsearch.SendKeys("browser.download.viewableInternally.enabledTypes");
            IWebElement buttonedit = driver.FindElement(By.ClassName("button-edit"));
            buttonedit.Click();
            IWebElement inputtext = driver.FindElement(By.XPath("/html/body/table/tr/td[1]/form/input"));
            inputtext.Clear();
            IWebElement buttonsave = driver.FindElement(By.XPath("/html/body/table/tr/td[2]/button"));
            buttonsave.Click();

            Thread.Sleep(1000);

Comment 12

[:Gijs (he/him)]

(In reply to StephenSoftware from comment #11)

Thanks to :Gijs (he/him) for the advice...

For all those who have the same problem below the workaround in C #.

😱

Does the selenium / webdriver wrapper not provide you with an easier way to change prefs? The reporter shared a config file where changing the pref is a one-line addition of a name=value pair...

Comment 13

[Cristian Nicolae (:xti)]

I am using the geckodriver + JAVA.

The workaround works on my PC, but partially on the Windows server where the Jenkins tests are executed with maven.

So I am still waiting for the fix of this issue.

Thank you!

Comment 14

[StephenSoftware]

Does the selenium / webdriver wrapper not provide you with an easier way to change prefs? The reporter shared a config file where changing the pref is a one-line addition of a name=value pair...

There are two option, you can use a default profile and in this case you can set all your preference, in the other way seleniun make a new empty, small, fresh and light profile with default values... I choosed the second option and for this reason I have to set the the preferences by hand...

Comment 15

[:Gijs (he/him)]

(In reply to Cristian Nicolae (:xti) from comment #13)

I am using the geckodriver + JAVA.

The workaround works on my PC, but partially on the Windows server where the Jenkins tests are executed with maven.

So I am still waiting for the fix of this issue.

I'm looking at this but it's not as straightforward as I might like...

Still, I'm pretty confused - if you can set the pref browser.helperApps.neverAsk.saveToDisk, why can't you set the pref browser.download.viewableInternally.enabledTypes in the same way? And what does "partially" mean?

To be clear, I'm asking in part because the mechanism by which you use these prefs is relevant - e.g. whether you have a clean new Firefox profile each time (apart from the prefs you set) or not, etc. - in terms of how we address this.

All of this is pretty weird because it would seem that what has happened here is:

1. once upon a time, 20(+) years ago, Firefox and/or its predecessors (mozilla suite/seamonkey, netscape, etc.), stored file handling preferences in the standard preference
2. they were migrated to a file called mimeTypes.rdf, also at least 12-15 years ago -- but they kept some migration code so those preferences still worked
3. they were migrated again to a file called handlers.json, about 2-3 years ago. Nobody knew about the migration code from (2) anymore.

and here we are today and the prefs are broken for some file types. There are no automated tests for them, nobody around will remember how they were supposed to work - we didn't even really know they existed until this report.

The migration code from (2) only reads the prefs if there is no data for the mimetype in question stored in (what is now) handlers.json . But in bug 1639067, we added data in that list by default for some mimetypes, including XML. So now the prefs get ignored.

A simple intervention here would be to check these preferences before adding such data to handlers.json in new profiles. I think that should help you, but I'm not really 100% sure, and what I definitely don't want to do is end up writing a patch and finding out weeks later that it was of no use to you.

Another option would be to always read these preferences, even if there is data in handlers.json. That would likely affect non-automation users, and it wouldn't be obvious to them why their Firefox suddenly started doing something different for some file types.

Really, from a complexity perspective, I think I'd prefer to just remove the 20-year old code, but I'd also prefer that you don't end up without a solution for what you're doing. So... what exactly are you using Firefox w/ selenium for ? Do you really just need all downloads to be saved to disk, irrespective of mimetype?

Comment 16

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #15)

A simple intervention here would be to check these preferences before adding such data to handlers.json in new profiles. I think that should help you, but I'm not really 100% sure, and what I definitely don't want to do is end up writing a patch and finding out weeks later that it was of no use to you.

In fact, thinking about it, it almost certainly won't help Stephen, based on the use of about:config to set these prefs, and assuming that's also how the neverAsk prefs are being set in their case...

Comment 17

[Cristian Nicolae (:xti)]

(In reply to :Gijs (he/him) from comment #15)

Still, I'm pretty confused - if you can set the pref browser.helperApps.neverAsk.saveToDisk, why can't you set the pref browser.download.viewableInternally.enabledTypes in the same way? And what does "partially" mean?

I will need to re-check my automated test, to understand why on my machine, all the xml downloads are working as expected with the new preference set, and why on Jenkins server (running on Windows), some xml are downloaded correctly, meanwhile other xml are not, and the Downloads Manager still pop-ups.

Really, from a complexity perspective, I think I'd prefer to just remove the 20-year old code, but I'd also prefer that you don't end up without a solution for what you're doing. So... what exactly are you using Firefox w/ selenium for ? Do you really just need all downloads to be saved to disk, irrespective of mimetype?

For each run, I am using a clean Firefox version (Serenity does it - check the configuration file attached).
Indeed, for all automated tests, I need a straight download to the set download directory location, no matter what is the file mime-type (in the same config file, you can see that I set a long list of mime-types on 'neverAsk' pref)

I don't want you to apply a risky fix, but rather use the best fix you consider, so my tests will pass again with geckodriver.

Thank you for helping all of us, affected by this change!

Comment 18

[:Gijs (he/him)]

(In reply to Cristian Nicolae (:xti) from comment #17)

(In reply to :Gijs (he/him) from comment #15)

Still, I'm pretty confused - if you can set the pref browser.helperApps.neverAsk.saveToDisk, why can't you set the pref browser.download.viewableInternally.enabledTypes in the same way? And what does "partially" mean?

I will need to re-check my automated test, to understand why on my machine, all the xml downloads are working as expected with the new preference set, and why on Jenkins server (running on Windows), some xml are downloaded correctly, meanwhile other xml are not, and the Downloads Manager still pop-ups.

If I had to guess, Windows can be peculiar about mimetypes it does/doesn't recognize, and/or the file extensions it associates with them (it likes, for instance, to pretend that image/jpeg mimetype files should have "jfif" extensions). This can sometimes throw off our download code, which tries to delegate to the OS for things like that (so that new installed apps get picked up if they add mimetype-extension mappings).

Really, from a complexity perspective, I think I'd prefer to just remove the 20-year old code, but I'd also prefer that you don't end up without a solution for what you're doing. So... what exactly are you using Firefox w/ selenium for ? Do you really just need all downloads to be saved to disk, irrespective of mimetype?

For each run, I am using a clean Firefox version (Serenity does it - check the configuration file attached).
Indeed, for all automated tests, I need a straight download to the set download directory location, no matter what is the file mime-type (in the same config file, you can see that I set a long list of mime-types on 'neverAsk' pref)

OK, thank you for clarifying!

Marco, what do you think about:

1. removing the code currently checking these old prefs
2. adding a boolean pref that gets treated as "always download"
3. writing migration code in BrowserGlue.jsm that checks for the "new profile + old pref is set" case, and sets the new pref from (2) ?

AIUI that will mean that :xti's usecase will "just work". For old profiles that had these prefs, I'm assuming / hoping that for filetypes that users actually encounter frequently we will have saved the results in the handlers DB already anyway so the pref is redundant - does that look right to you?

Comment 19

[Cristian Nicolae (:xti)]

I was reviewing the tests logs and I still don't understand.

On my PC (Windows 10), all related tests are running fine and the XML, PNG and PDF files are downloaded directly with the workaround preference set (I set 'browser.download.viewableInternally.enabledTypes=svg,webp,avif').

On the Jenkins sever (same Windows 10), where exactly the same code is executed, it works only for the first XML download and then, it fails for the rest of the tests where XML and PNG files are downloaded. However, all PDF related tests are passing.

Both runs of the full test suite (downloads) were executed on the same browser session. Driver is not reset between the tests, so the configuration set by Serenity at the very beginning, remains until the end.

On both machines is running the same Firefox v82.0.1 x64. Very strange behavior....

Comment 20

[Marco Bonardo [:mak]]

Searching around the Web, these prefs seem indeed to be used and suggested widely and mostly for automation.
There are a bunch of automation tutorials and StackOverflow answers about them, it's clear they must keep working somehow.
I think for our final users these are not critical, nor necessary. Historical users that we may surprise, are likely able to fix handlers in prefs.

I must note there is not just browser.helperApps.neverAsk.saveToDisk, but also browser.helperApps.neverAsk.openFile, migrating one while leaving the other one untouched would be confusing.
The new general pref should cover both of those, and work like "don't dare to show a dialog".

We have 2 concerns here (afaict):

1. does migrating only new profiles cover most of the cases? Can we assume all automation starts from new profiles?
   From my searches in general these frameworks always start with a new FirefoxProfile(), and testing usually prefers a clean slate. Cases where an existing profile are used, could well set the new pref in that base profile and fix everything with a one-line change.
2. Is there the need for these prefs to be fine-grained, so allow to specify certain types but not all of them?
   Assuming we only care about automation, I think global boolean prefs will do. Some rare cases where automation wants the dialog for something but not something else, will have to be fixed, but that sounds more like "testing the dialog", why would one outside of Mozilla do it?

I think the plan you made may work.
When we migrate, I'd suggest to not remove the old prefs, that will let us eventually go back if we missed something critical.

Comment 21

[Cristian Nicolae (:xti)]

This issue is not reproducible anymore on my side on the latest Firefox version v.85.0.2.

Was it fixed by another improvement/bug-fix?

Should I close the ticket?

Thank you!

Comment 22

[StephenSoftware]

Yes, for me you can close the ticket...
I solved putting at empty string with the following statement:
firefoxOption.SetPreference("browser.download.viewableInternally.enabledTypes", "");
Thanks to everybody for help!
Stefano

Comment 23

[Cristian Nicolae (:xti)]

Thank you too, for fixing it.

We can resume related automated tests.

Closing ticket!

Comment 24

[:Gijs (he/him)]

So, confusingly, I am not sure what would have fixed this... We have a separate resolution status for this. I'm also wondering if we need to add some tests for this on our side (a bit tricky because the majority of our tests run on pre-initialized profiles)...

If it's possible for someone to determine what change in nightly fixed this, that would be helpful!