1180126 - Content-Disposition: inline; filename= doesn't work with Save As

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Rikisz]

Attachment: Beaver.jpg

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:39.0) Gecko/20100101 Firefox/39.0
Build ID: 20150630154324

Steps to reproduce:

1. Click on image link
2. Right click on image select Save Image As...
3. File name shown is ...


Actual results:

File name suggested in save dialog is ...


Expected results:

Suggested file name should be Beaver.jpg

Comment 1

[Rikisz]

Steps to reproduce:

1. Right click on image link Beaver.jpg
2. Select Open Link in New Window
3. Right click on image 
4. Select Save Image As...

Actual results:

File name suggested in save dialog is "attachment.cgi.jpg"


Expected results:

Suggested file name should be "Beaver.jpg".

Also Ctrl+S shows "Beaver.jpg".

It's regression from previous versions Firefox 38.0.5, 38.0

Comment 2

[Rikisz]

HTTP headers show:

Connection: Keep-Alive
Content-Disposition: inline; filename="Beaver.jpg"
Content-Length: 20717
Content-Type: image/jpeg; name="Beaver.jpg"; charset=
Date: Fri, 03 Jul 2015 08:05:57 GMT
Keep-Alive: timeout=5, max=1000
Server: Apache
X-Backend-Server: web3.bugs.scl3.mozilla.com
X-Content-Type-Options: nosniff
X-XSS-Protection: 1; mode=block

Comment 3

[Loic]

Regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=90adc073cbc6&tochange=421548077f12

Comment 4

[Alice0775 White]

I got a different range

Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=2e9b6048bd0f&tochange=f687743cc499

Suspect: Bug 1139804

Comment 5

[Alice0775 White]

Via local build
Last Good: fcaed7860ea4
First Bad: 3f5eae4fac0f+f687743cc499

Regressed by: Bug 1139225

Comment 6

[Alice0775 White]

[Tracking Requested - why for this release]: Regressed by Bug 1139225 since Firefox39

Comment 7

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

This is a regression which will impact a lot of our end-users. Adding a tracking flag for FF40, FF41 and FF42.

Comment 8

[Seth Fowler [:seth] [:s2h]]

Attachment: Read content disposition regardless of content type in imgRequest::PrepareForNewPart

Unfortunately in bug 1139804 I mistakenly added the line that reads the content
disposition to a codepath that only runs if MIME type sniffing fails. It needs
to be pulled back out to the top level.

Comment 9

[Seth Fowler [:seth] [:s2h]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e229f27554e1

Comment 10

[Seth Fowler [:seth] [:s2h]]

Thanks for the quick review!

Comment 11

[Seth Fowler [:seth] [:s2h]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/baad399c214a

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/baad399c214a

Comment 13

[Jefferson]

Any possibility of uplift?

A user reported this on SuMo today: https://support.mozilla.org/questions/1071457

A temporary workaround is:

(1) right-click > View Image (or right-click > Ctrl+View Image for a new tab)
(2) Save Page As

Comment 14

[Kate Glazko]

Hi Seth, can you fill out the uplift requests? We want it for Beta/Aurora.

Comment 15

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 8631380 [details] [diff] [review]
Read content disposition regardless of content type in imgRequest::PrepareForNewPart

Approval Request Comment
[Feature/regressing bug #]: 1139225
[User impact if declined]: Wrong filename displayed when right clicking on some images and choosing "Save Image As...".
[Describe test coverage new/current, TreeHerder]: On m-c for a day. We'd like a test for this, but I'm unfortunately not sure how to write one. I'll file a followup.
[Risks and why]: Low risk. The change is very small.
[String/UUID change made/needed]: None.

Comment 16

[Kate Glazko]

Comment on attachment 8631380 [details] [diff] [review]
Read content disposition regardless of content type in imgRequest::PrepareForNewPart

Approving uplift to Aurora and Beta because low risk/small change, has not caused issues on m-c.

Comment 17

[Jan Henning [:JanH]]

On a side note, I've found that with e10s enabled this bug goes back much further, to September 2013 at least, if not earlier - on earlier Nightly builds, amongst other things the context menu doesn't work with e10s, so I can't really test.

In a current Nightly, Seth's patch has fixed the problem both with and without e10s enabled, so all is well.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7e9b8c0e22db

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d2684ca58fcc

Comment 20

[Seth Fowler [:seth] [:s2h]]

(In reply to JanH from comment #17)
> On a side note, I've found that with e10s enabled this bug goes back much
> further, to September 2013 at least, if not earlier - on earlier Nightly
> builds, amongst other things the context menu doesn't work with e10s, so I
> can't really test.
> 
> In a current Nightly, Seth's patch has fixed the problem both with and
> without e10s enabled, so all is well.

I haven't investigated this issue specifically, but bug 1139804 was part of a series of patches that fixed *tons* of races in the image loading code. That's the sort of issue that e10s might tickle. So maybe there was a race related to content-disposition that bug 1139804 fixed, and that would've been the end of the issue, except that I made a mistake in bug 1139804 which was fixed here.

Comment 21

[Nazir Ahmed Sabbir [:NaSb]]

Reproduced with 42.0a1 (2015-07-03) (Build ID: 20150703030216) on Linux x64 by following comment 1's instruction!

The bug is fixed on Latest Nightly 42.0a1 (2015-07-15) (Build ID: 20150715030207),

Latest Aurora 41.0a2 (2015-07-15) (Build ID: 20150715004006) and

Latest Beta 40.0b3 (Build ID: 20150713153304)

Mozilla/5.0 (X11; Linux x86_64; rv:42.0) Gecko/20100101 Firefox/42.0
Mozilla/5.0 (X11; Linux x86_64; rv:41.0) Gecko/20100101 Firefox/41.0
Mozilla/5.0 (X11; Linux x86_64; rv:40.0) Gecko/20100101 Firefox/40.0

Comment 22

[Rezaul Huque Nayeem]

Reproduced with 42.0a1 (2015-07-03) (Build ID: 20150703030216) on windows 10 x64 by following comment 0 instruction!

The bug is fixed on Latest Nightly 42.0a1 (2015-07-21) (Build ID: 20150721004010),
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:42.0) Gecko/20100101 Firefox/42.0

Latest Aurora 41.0a2 (2015-07-15) (Build ID: 20150715004006),
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:41.0) Gecko/20100101 Firefox/41.0

Latest Beta 40.0b6 (Build ID: 20150720220238)
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:40.0) Gecko/20100101 Firefox/40.0

[bugday-20150722]

Comment 23

[Nazir Ahmed Sabbir [:NaSb]]

It is verified on Windows and Linux (Comment 21 and Comment 22), Marking it as verified!

Comment 25

[Antonio Ospite]

Hi,

I still experience this issue with Fixefox 49, it happens for instance on any "attachment" link on the Debian bug tracker, e.g.:

https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=704037;filename=piuparts_binfmt-misc_umount_fail.log;msg=5

from https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=704037

One could argue that they should use "Content-Disposition: attachment" instead of inline (and I'll propose that), but other browsers seem to be able to get the file name right when saving these links.

If you confirm the issue, can this bug be re-opened or should I file another one?

Thanks,
   Antonio

Comment 26

[Loic]

(In reply to Antonio Ospite from comment #25)
> Hi,
> 
> I still experience this issue with Fixefox 49, it happens for instance on
> any "attachment" link on the Debian bug tracker, e.g.:
> 
> https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=704037;
> filename=piuparts_binfmt-misc_umount_fail.log;msg=5

WFM with FF49 or 52, see http://i.imgur.com/LkgmTpw.jpg
 
> If you confirm the issue, can this bug be re-opened or should I file another
> one?

Yes, this bug report is closed, you have to file a new one, please.

Comment 27

[Antonio Ospite]

(In reply to Loic from comment #26)
> (In reply to Antonio Ospite from comment #25)
> [...]
> WFM with FF49 or 52, see http://i.imgur.com/LkgmTpw.jpg

I am on linux and the filename detection works when I left-click or middle-click on the link, but it doesn't if I right-click and choose "Save as".

> > If you confirm the issue, can this bug be re-opened or should I file another
> > one?
> 
> Yes, this bug report is closed, you have to file a new one, please.

OK, I will, thanks.