1759916 - Attribute "target" ignored when exists also "download" attribute

Status: RESOLVED WONTFIX

(Firefox :: File Handling, defect, P2)

Description

[Kamil]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/99.0.4844.51 Safari/537.36

Steps to reproduce:

I create link to PDF file with two attributes "target="blank" and download="file.pdf". When "href" of link is to same domain the file is opened in same tab. The "target" attribute is ignored. When "href" of link is to another domian the file is correctly open in new tab.

We can imagine that we are on page: www.first-domian.com
This link will open new tab.
<a href="http://second-domian.com/file.pdf" target="blank" download="File.pdf"">Download FILE</a>

These examles open links in same tab
<a href="http://first-domian.com/file.pdf" target="blank" download="File.pdf"">Download FILE</a>
<a href="/documents/file.pdf" target="blank" download="File.pdf"">Download FILE</a>

Actual results:

The "target" attribute is ignored when link direct to same domain and link has attribute "download".

Expected results:

The "target" attribute should not to be ignored when link direct to same domain and link has attribute "download".

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

[:Gijs (out until Jan 5th; he/him)]

Neil, do you think you could look into this?

Comment 3

[Piotr Olczak]

Our web app's control flow is also heavily affected by this Firefox 98 change. I agree it's not really a good design to require the user to click the PDF link before allowing him to advance, but it's something our client specifically asked us to do.

From my testing it looks like neither of the mentioned target="blank" (i.e. open in a frame named 'blank' and if it doesn't exist create it) or the more common target="_blank" (i.e. open in a new frame) work as intended.

I'm also not sure if the usage of the "download" attribute matters here. If you have a link:
<a href="../pdfs/file.pdf" target="_blank">Test</a>
it should probably work the same as:
<a href="../pdfs/file.pdf" download target="_blank">Test</a>
since the PDF should probably already be classified as a download even without hinting at it (though that's only my guess)

Comment 4

[Kamil]

We made tests and if attribute "download" not exists on link, the PDF is opening in new TAB.

Comment 5

[Marco Bonardo [:mak]]

This seems worth investigating sooner than later as it may be breaking website state.

Comment 6

[Neil Deakin]

In the case where the download link points to a different host:
<a href="http://otherdomain.com/file.pdf" target="blank" download="File.pdf"">Download FILE</a>

• the download attribute is ignored, so the pdf opens in the specified target window.

In the case where the download link points to the same host:
<a href="file.pdf" target="blank" download="File.pdf"">Download FILE</a>
the target is ignored, and the file is downloaded. The default behaviour is to open pdf files internally when downloading them.

The target is ignored when a download filename is specified at https://searchfox.org/mozilla-central/rev/1f617334179cf28b4b310d1d116ddbc8ef3348ea/dom/base/nsContentUtils.cpp#5617 Some small commentary on this behaviour is at https://bugzilla.mozilla.org/show_bug.cgi?id=676619#c71

We could add a special case and make it so that pdf files when set to open internally do use the target if desired. Note though that this isn't specific to pdf files; other files such as html and images also ignore the target when set as a download. If we did do so this, I think we would need to add some of the code of nsURLLoader that checks the preferred action to be added into nsContentUtils::TriggerLink

Comment 7

[Neil Deakin]

Tom, since you added this code in nsContentUtils::TriggerLink, do you have any insight as to what we should do here? Is there some security reason I'm missing, or would it be ok to add a special case for pdf here, or remove the blanking out of the target in TriggerLink and instead pass the target onwards and handle it later on?

Comment 8

[Tom S. (please needinfo tschuster)]

It really has been a long time, so I don't remember much. But if we don't remove the target attribute, doesn't that mean we would unnecessarily open a new tab only to show a download dialog? I don't think that we made this decision based on security however.

Comment 9

[Kamil]

If you remove "target" you take away The programmer possibility to define what browser should make with link. I think it if new tab will be open or not should be at programmer decision, not from internal code of the browser.

Comment 10

[:Gijs (out until Jan 5th; he/him)]

(In reply to Tom Schuster [:evilpie] from comment #8)

It really has been a long time, so I don't remember much. But if we don't remove the target attribute, doesn't that mean we would unnecessarily open a new tab only to show a download dialog?

We already do that in many cases, and there's logic to automatically close such tabs/windows (I should know, I broke an edgecase related to it in the past and fixed it in bug 1678965). Also, we don't really show a dialog anymore on 98+, we just start the download by default and show the download panel.

So basically, I don't think this should be a concern.

I don't think that we made this decision based on security however.

Thanks for confirming.

(In reply to Neil Deakin from comment #6)

We could add a special case and make it so that pdf files when set to open internally do use the target if desired. Note though that this isn't specific to pdf files; other files such as html and images also ignore the target when set as a download. If we did do so this, I think we would need to add some of the code of nsURLLoader that checks the preferred action to be added into nsContentUtils::TriggerLink

I think this would be great. I also think we should consider defaulting to _blank if no target is given, for the PDF + internal handling case, for the reasons folks have elaborated on, uh, extensively, in 1756980.

Neil, are you able to work on this in the near future?

Comment 11

[Neil Deakin]

Actually I'm not sure if this is possible to implement this way. The target window is opened first, long before the uri has started loading so we don't even know the content type yet.

Comment 12

[Mathew Hodson]

Since bug 1756980 was fixed, should this be marked "won''t fix" now? It seems like the idea is to continue to ignore "target" and use the logic from bug 1756980 instead to determine if a new tab is needed.

Comment 13

[:Gijs (out until Jan 5th; he/him)]

(In reply to Mathew Hodson from comment #12)

Since bug 1756980 was fixed, should this be marked "won''t fix" now? It seems like the idea is to continue to ignore "target" and use the logic from bug 1756980 instead to determine if a new tab is needed.

Perhaps. I'm still wondering if, if the target is _blank, we can open the window and then close the tab/window again if it stays blank and we do end up not displaying anything, similar to downloads more generally - Nika, thoughts?

Comment 14

[Neil Deakin]

The behaviour now when a link has both the target and download attributes set is:

When pdf is set to 'open internal':

• For a link to the same site, open in a new tab -- this seems to ignore the specified target and always open in a new tab.
• For a link to a different site, open in the target tab.

When pdf is set to 'save as':

• For a link to the same site, the pdf is saved and the download panel is opened. Neither the page nor a new tab is opened.
• For a link to a different site, open in the target tab. The pdf is also saved and the download panel is opened. If the target tab is already open, the page is unchanged and the tab is switched to. If the target tab is not already open, the tab opens and the quickly closes again.

Similar behaviour occurs when the setting to open in a new window instead of a tab is set, except with a new window.

If this is the expected behaviour, then we can close this bug.

Comment 15

[Nika Layzell [:nika] (ni? for response)]

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

(In reply to Mathew Hodson from comment #12)

Since bug 1756980 was fixed, should this be marked "won''t fix" now? It seems like the idea is to continue to ignore "target" and use the logic from bug 1756980 instead to determine if a new tab is needed.

Perhaps. I'm still wondering if, if the target is _blank, we can open the window and then close the tab/window again if it stays blank and we do end up not displaying anything, similar to downloads more generally - Nika, thoughts?

That sounds like a distinctly worse experience than the existing experience, where we open a pop-up when we won't actually need it, and then abruptly close it. I'm vaguely inclined not to do that and continue ignoring target attributes on download links.

Based on a quick look at chromium's behaviour this also seems more similar to what they do with checking the download attribute and switching to a download (https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_anchor_element.cc;l=445;drc=f26bb66a0f935f7c677cbc47ff809c13968b4db3), before they check the target attribute at all: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_anchor_element.cc;l=500;drc=f26bb66a0f935f7c677cbc47ff809c13968b4db3.

Comment 16

[:Gijs (out until Jan 5th; he/him)]

OK, then I think we should wontfix this bug in favour of bug 1756980 (which I think still addresses the original concern).