1807260 - Setting Windows to open a file Firefox can't open with Firefox (e.g. epub) causes a loop continuously opening infinite new tab pages

Status: VERIFIED FIXED

(Firefox :: File Handling, defect, P3)

Description

[QLS]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:108.0) Gecko/20100101 Firefox/108.0

Steps to reproduce:

1. Opened EPUB file Open with... and picked Mozilla Firefox.
2. Firefox window showed up.
3. Save as... prompt opened automatically.
4. Clicked Open with... Firefox (in browser).

Actual results:

Many tabs started opening and would keep opening until my RAM was depleted. When I closed Firefox and opened it again the tabs kept opening. I had to remove the EPUB file completely from the location I opened it from.

Expected results:

1. Cannot open the file OR
2. Propose an extension to read EPUB files in Firefox.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Firefox::File Handling' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Hello, thank you for the bug report!
I managed to reproduce this issue on:

• Nightly 110.0a1;
• Firefox 109.0b6;
• Firefox 108.0.1;

Tested and reproduced on:

• Windows 10;
• Ubuntu 22;
• macOS 12;

Setting as NEW so that the developers can have a look.

Comment 3

[:Gijs (out for now; he/him)]

So I don't think this is S2, as this likely doesn't affect a lot of people (requires non-default configuration as downloads now save to disk by default, using epub files and presumably finding a custom executable on disk, and I think also clicking "always do this for this type of file", though the steps don't list that? Without that I just get shown the same dialog again), but I'd like to get to the bottom of this.

I have repeatedly tried to fix this type of issue. (bug 1678255, bug 1633790, bug 1496380, bug 1750253)

I cannot reproduce this myself, on Windows, even if I tick "always do this for this type of file" - I get prompted again. In order to investigate why the fixes for this issue aren't working for you, I'm going to need some more info.

Can you link to an example epub where this reproduces? I tried to use https://filesamples.com/formats/epub but cannot reproduce with that.

Do you have an epub reader installed?

Can you provide a screenshot of the dialog you get, and the options in the dropdown? (you can probably change the setting for epubs in the Firefox settings' if you search for "files", back to "always ask" to see the dialog again)

How are you selecting Firefox as the app to open with? And do you have more than 1 version of Firefox installed on the machine where this is happening?

Comment 4

[QLS]

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

Can you link to an example epub where this reproduces? I tried to use https://filesamples.com/formats/epub but cannot reproduce with that.

I've used the same example files in recording linked to this comment.

Do you have an epub reader installed?

I don't.

Can you provide a screenshot of the dialog you get, and the options in the dropdown? (you can probably change the setting for epubs in the Firefox settings' if you search for "files", back to "always ask" to see the dialog again)

Recordings linked. (rec1 and rec2.mp4)

How are you selecting Firefox as the app to open with? And do you have more than 1 version of Firefox installed on the machine where this is happening?
Save As or Open With and then Remember the choice. My interface on the recordings is Polish but I'm sure you'll be able to figure it out.

Comment 5

[QLS]

Attachment: rec1.mp4

Comment 6

[QLS]

Attachment: rec2.mp4

This item shows how to reproduce the bug when no setting for EPUB file is present in Firefox settings.

Comment 7

[:Gijs (out for now; he/him)]

(In reply to QLS from comment #6)

This item shows how to reproduce the bug when no setting for EPUB file is present in Firefox settings.

This shows Firefox is the default app for epub files on your machine. This is not the case for me, and Firefox has no facilities to make this happen. I expect the difference on your machine is because it was associated to such files in the Windows registry (perhaps with Windows explorer's "open with" dialog). Is that possible?

Comment 8

[QLS]

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

I expect the difference on your machine is because it was associated to such files in the Windows registry (perhaps with Windows explorer's "open > with" dialog). Is that possible?
Yes, you are right. It is listed as step 1. in the Reproduce comment (the first one) - meaning it won't happen if you don't specifically do the Open with... part. What's curious is the new tab opening cascade after you manage to set it as possible option for opening.

Comment 9

[:Gijs (out for now; he/him)]

(In reply to QLS from comment #8)

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

I expect the difference on your machine is because it was associated to such files in the Windows registry (perhaps with Windows explorer's "open > with" dialog). Is that possible?

Yes, you are right. It is listed as step 1. in the Reproduce comment (the first one) - meaning it won't happen if you don't specifically do the Open with... part.

Ah, sorry - I assumed this was referring to "open with" in Firefox's own dialog...

What's curious is the new tab opening cascade after you manage to set it as possible option for opening.

It's because we hand the file to the operating system for opening (ShellExecute) and the operating system hands the file back to us, opening a new tab, at which point we hand the file to the OS again, etc. etc.

Comment 10

[QLS]

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

(In reply to QLS from comment #8)

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

I expect the difference on your machine is because it was associated to such files in the Windows registry (perhaps with Windows explorer's "open > with" dialog). Is that possible?

Yes, you are right. It is listed as step 1. in the Reproduce comment (the first one) - meaning it won't happen if you don't specifically do the Open with... part.

Ah, sorry - I assumed this was referring to "open with" in Firefox's own dialog...

What's curious is the new tab opening cascade after you manage to set it as possible option for opening.

It's because we hand the file to the operating system for opening (ShellExecute) and the operating system hands the file back to us, opening a new tab, at which point we hand the file to the OS again, etc. etc.

Ok, got it. In this case - should Firefox alert the user that it cannot open the file instead of handing the opening over to the OS? But it also begs the question - will that kind of a patch break the possibility to save files if not supported files type is accessed with Firefox via link?

Comment 11

[:Gijs (out for now; he/him)]

(In reply to QLS from comment #10)

Ok, got it. In this case - should Firefox alert the user that it cannot open the file instead of handing the opening over to the OS?

Ideally yes. At a minimum, it should reshow the prompt, which is what it is supposed to do. The code for that is here. We've avoided specific warning UI or similar because it is difficult to be 100% certain that this is what is happening, and falling back to asking (as opposed to automatically doing what the user said - hand file to the OS default) leaves the user with agency as to what to do next, which addresses your next point:

But it also begs the question - will that kind of a patch break the possibility to save files if not supported files type is accessed with Firefox via link?

Right, if we just showed a warning with no option for the user to do something then that would break all normal downloads. That is exactly the problem - we have to figure out that the OS default for the filetype is "us", and then stop it from opening automatically again and again. The code I linked above is supposed to be doing exactly that. When I checked on my Windows machine earlier after my last comment, from JS, the IsCurrentAppOSDefault helper was returning the correct result (true if Firefox is set as the default epub app). So presumably either the code I linked is doing something else wrong or at that point the helper is returning a different value. I'll need to debug in some more detail, but it's tricky because I can't seem to figure out how to make a local, debuggable build the OS default - if I select a different firefox.exe, Windows still hands the file to the "main" Firefox I have installed and use for day-to-day browsing etc...

For the external protocol case (mailto and similar) we have also added another fallback protection which notices that we keep opening the same URL and then stops rather than just mindlessly continuing. This means you end up with 2 tabs + 1 dialog instead of 1 tab with a dialog, but it's of course still better than the loop...

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:Gijs, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 13

[:Gijs (out for now; he/him)]

I still need to get back to this, and I hope to do so next week or the week after. But marking as P3/S3 for now.

Comment 14

[:Gijs (out for now; he/him)]

(doing some bug flag admin; we can't fix this in Firefox releases that have already shipped so marking those wontfix. Obvs. will still try to fix this in a future release.)

OK, I made some time to dive into this.

It seems, as is somewhat usual for stuff like this, that there is a cascade of unfortunate coincidences here.

When we load a local file (this is easier to debug in the first instance than loading from the web, and we'd hit this case eventually anyway, if there is a loop), we end up here

nsCOMPtr<nsIMIMEInfo> mimeInfo = ValidateFileNameForSaving(
    fileName, aMimeContentType, uri, nullptr, flags, allowURLExtension);

That calls into ValidateFileNameForSaving to obtain a mime info object. That object gets passed to the nsExternalAppHandler constructor a bit later

// NB: ExternalHelperAppParent depends on this listener always being an
// nsExternalAppHandler. If this changes, make sure to update that code.
nsExternalAppHandler* handler = new nsExternalAppHandler(
    mimeInfo, extension, aContentContext, aWindowContext, this, fileName,
    reason, aForceSave);

Somewhere in there, it uses the mime info object with the API I mentioned in my previous comment to determine if we're the default app. That fails (ie returns false), at least on Windows, because nothing has ever set the default application for the mimetype. This normally happens in nsOSHelperAppService::GetByExtension

mimeInfo->SetDefaultDescription(defaultDescription);
mimeInfo->SetDefaultApplicationHandler(defaultApplication);

But we never invoke that for this mimetype. Why?

Well, because ValidateFileNameForSaving uses the more high level GetFromTypeAndExtension mime service API - here: https://searchfox.org/mozilla-central/rev/c5eeb9f4fcfe52b028e7774054444f2982feccc3/uriloader/exthandler/nsExternalHelperAppService.cpp#3328,3338,3351,3360

Here the first oddity happens: it never passes the extension. This is because we don't normally trust URL extensions (think php and such like for scripts serving downloads). This is bad because now we have to find all the info using just the mimetype. Windows is bad at this. Last time I checked, it had never heard of zip files (or, a bit less hyperbolically, at least thought they shouldn't have application/zip as the mimetype - ref bug 1609466).

Anyway, so here we are, in GetFromTypeAndExtension, without an extension.

The very first thing that does is defer to the OS-specific implementation. On Windows, that asks the registry for a file extension given the mimetype. That fails (as an aside: this is odd because we probably got the mimetype from windows on the basis of the filename in the first place, so the fact that the reverse operation doesn't work is confusing). Now we have no file extension, so it bails out.

(at this point, we've already lost the chance of ever hitting the Windows-specific lower level GetByExtension API)

OK, but the ValidateFileNameForSaving function has a thing that bails out if we have no file info and provides the extension anyway, right?

Well yes, but we're in GetFromTypeAndExtension, and it's gonna do its darndest to get us some useful information. So it consults a Firefox-hardcoded list, and the local handler info. The latter has info, because you saved "always open files like this with [the system app]". So now it finds an extension there and considers its job done. We return to ValidateFileNameForSaving which does some other stuff, and end up with a mime info that doesn't have the relevant info to determine if we are the default app, so it just defaults to "no" and lets us open the file again, and again, and again.

--

The obvious question is "how do we fix this".

Making ValidateFileNameForSaving pass the extension in more cases will help. It seems uncontroversial to do this for the local file case. But that still won't help for web-served epub (at least not immediately - by the second roundtrip to the OS we'll have a file URL and will ask the user anyway). It also seems a little wall-papery. The root issue is that we end up with a mime info object that doesn't have the right information.

Still, it seems relatively easy to do that, and the FillHandlerInfo and friends could land other consumers of the mime service / external helper app service in the same hot water.

Molly, how terrible an idea would it be to effectively fetch the default app description from the registry etc. "just in time", ie have a flag that tracks whether it's happened since the primary extension / mimetype last changed, and if it hasn't, (re-)fetch it when anyone asks for the default app description, default application, or anything using those fields? So something like bool mHasRegistryInfo, and an UpdateRegistryInfo into which we factor out all of this stuff and then call that from anything relying on mDefaultApplication, mDefaultAppDescription, etc.?

(I'd also be open to an even more thorough refactoring here, I just don't know exactly what that would look like.)

Comment 15

[:Gijs (out for now; he/him)]

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

Molly, how terrible an idea would it be to effectively fetch the default app description from the registry etc. "just in time", ie have a flag that tracks whether it's happened since the primary extension / mimetype last changed, and if it hasn't, (re-)fetch it when anyone asks for the default app description, default application, or anything using those fields? So something like bool mHasRegistryInfo, and an UpdateRegistryInfo into which we factor out all of this stuff and then call that from anything relying on mDefaultApplication, mDefaultAppDescription, etc.?

(I'd also be open to an even more thorough refactoring here, I just don't know exactly what that would look like.)

Hm, I guess at least one other option would be keeping that code in nsOSHelperAppService but ensuring we call it from nsExternalHelperAppService::GetFromTypeAndExtension after determining type + extension, even if we couldn't get that out of the registry...

Comment 16

[Molly Howell (she/her) (mostly inactive)]

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

Molly, how terrible an idea would it be to effectively fetch the default app description from the registry etc. "just in time", ie have a flag that tracks whether it's happened since the primary extension / mimetype last changed, and if it hasn't, (re-)fetch it when anyone asks for the default app description, default application, or anything using those fields? So something like bool mHasRegistryInfo, and an UpdateRegistryInfo into which we factor out all of this stuff and then call that from anything relying on mDefaultApplication, mDefaultAppDescription, etc.?

I don't see any issue here. Were you worried about doing a whole bunch more registry reads? The OS can handle that, I don't think it would be a performance problem (and it probably won't be any disk I/O). My preference, as you know, would be to not really rely on MIME types for much of anything on Windows; they were never really part of the operating system or the subject of any deep or lasting support there, so (as you observed with .zip) we can't count on getting a useful amount of information from or about them.

Comment 17

[Matty]

I see no mention of XML in this thread so I wanted to point out that this is an issue for multiple types of files. I just experienced this issue after attempting to open a downloaded atom feed XML document and mistakenly selecting firefox in the "Open with" dialog shown when clicking an item in the Ctrl+Shift+Y download status flyout. This resulted in tabs continuously being opened until I killed the window. This was on Fedora 36 with Firefox 109.01.1.

Comment 18

[:Gijs (out for now; he/him)]

(In reply to Matty from comment #17)

I see no mention of XML in this thread so I wanted to point out that this is an issue for multiple types of files. I just experienced this issue after attempting to open a downloaded atom feed XML document and mistakenly selecting firefox in the "Open with" dialog shown when clicking an item in the Ctrl+Shift+Y download status flyout. This resulted in tabs continuously being opened until I killed the window. This was on Fedora 36 with Firefox 109.01.1.

The specifics here do not depend on the file being XML or otherwise. They do depend on:

• the OS and what it knows about mimetypes vs. extensions
• what action you have saved in Firefox as the default thing to do with a given filetype
• what the file extension on a file is
• what mimetype the server sends the file as
• what mimetype the OS reports the file as when opened locally

Your comment doesn't contain enough information for me to be sure about what exactly happens in your case. However, given that it is happening on Linux, the fixes I'm making here are unlikely to address your issue. Please file a separate report with more details (ideally, a link to a file/page that reproduces the issue, and if you feel comfortable with it, your handlers.json from your Firefox profile. We'll probably need to investigate what mimetype your OS reports for the file, too, but let's do that in a separate bug.)

Comment 19

[:Gijs (out for now; he/him)]

Attachment: Bug 1807260 - refactor access to mDefaultApplication from callers, r?mhowell

This makes it easier to ensure it's up-to-date when called,
rather than relying on member access.

Comment 20

[:Gijs (out for now; he/him)]

Attachment: Bug 1807260 - ensure nsMimeInfoWin's default app information is always-up-to-date, r?mhowell

Depends on D169711

Comment 21

[:Gijs (out for now; he/him)]

Attachment: Bug 1807260 - ensure local files opened have their extensions taken into account when determining what to do with them, r?mhowell

Depends on D169712

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0844daad0d31
refactor access to mDefaultApplication from callers, r=mhowell
https://hg.mozilla.org/integration/autoland/rev/3c5e8098c9a8
ensure nsMimeInfoWin's default app information is always-up-to-date, r=mhowell,necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/99d0e5f084cf
ensure local files opened have their extensions taken into account when determining what to do with them, r=mhowell

Comment 23

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/0844daad0d31
https://hg.mozilla.org/mozilla-central/rev/3c5e8098c9a8
https://hg.mozilla.org/mozilla-central/rev/99d0e5f084cf

Comment 24

[Ina Popescu, Desktop QA]

I've reproduced the issue on Nightly 112.0a1 (2023-02-18) on Windows 10.
The issue is no longer reproducible on Nightly 113.0a1 (2023-04-04) on the same system.
I've attached a screen recording showing how the epub files are handled now by Nightly.

Comment 25

[Ina Popescu, Desktop QA]

Attachment: Open epub files_fixed.mp4