130993 - option+click/shift+click to save a file saves a version from the cache, not the most recent version on the server

Status: NEW

(Firefox :: File Handling, defect)

Description

[sairuh (rarely reading bugmail)]

thanks to Simon for pointing this out. i can repro this using 2002.03.13.08 comm
trunk bits on mac 10.1.3. will test on other platforms in a bit...

seems like option+click on a link to save it will actually saved a cached
version of a file, not the latest version that's currently on the server. note:
using "save as link" from the context menu does work as expected, however.

recipe: you'll need to have access to a server to modify a test file.

1. created a simple text file (eg, http://hopey.mcom.com/tests/cache-test.txt).
2. went to http://hopey.mcom.com/tests/ (html directory listing), and
option+clicked the link for "cache-test.txt", then saved it to the desktop.
3. view the file --looks fine since it hasn't been modified yet.
4. edit the file on the server side so that it differs from the version in step (1).
5. go back to or reload http://hopey.mcom.com/tests/ --but don't hit shift+reload.
6. repeat step (2) to save it.
7. view this most recently.

results: the page saved is the "before changes" one in step (1), not the most
recently modified one in step (2).

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch to fix

Comment 2

[sairuh (rarely reading bugmail)]

yep, affects all platforms (eg, shift+clicking a link on linux and win2k).

found out that shift-reloading doesn't help as a workaround either.

two workarounds: using "save as link" from context menu, or:

1. view the newly modified page in mozilla/n6.x
2. go back to dir listing, then use option/shift+click to save the link

Comment 3

[sairuh (rarely reading bugmail)]

darin, law et al., r/sr?

Comment 4

[Felix Miata]

Let's not automatically download the new version if the page has changed in the
few seconds since finishing the one present in the cache, particularly if images
are involved. If that has happened, a dialog should offer a choice.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> Let's not automatically download the new version if the page has changed in
> the few seconds since finishing the one present in the cache, particularly if
> images are involved.

Images are completely irrelevant to the operation at hand.  Generally when one
is doing "save link as" one has not yet downloaded the file, so the assumption
that what's wanted is what's on the server as opposed to what's in the browser
cache (possibly from months ago) is a good one.

A case could be made for doing a "normal" load (ie the same as a regular load of
the URI would do) with the cache validation normally involved and such, I guess.
 But we should make "save link as" and "option-click" do the same exact thing.

Whichever we do, we should not have a new dialog here.  If I do "save link as" I
want to save, not to look at dialogs.

Comment 6

[Tuukka Tolvanen (sp3000)]

Click a link to some large media file that takes ages to download, use helper
app to view it. Decide you want to save it; go to link, (shift|option)-click on
link, save. Unless I'm missing something here, the proposed behavior would not
allow fetching the file from cache?

Comment 7

[Boris Zbarsky [:bzbarsky]]

That's correct...  but most halfway-decent helper apps have a "save" option. 
The only exception I know of is Acrobat Reader.

But that's a good point.  Like I said, it may make more sense to do a
LOAD_NORMAL and let normal cache-validation stuff kick in...

Comment 8

[Bill Law]

Yes, a normal load is called for.  Otherwise, we'll be dealing with the
"shift-click hits the server when the data is already in the cache" bug.

It looks like that will require work in contentAreaUtils.js so it doesn't force
a choice between "always from cache" and "never from cache," correct?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Yeah. It should be a choice between "always from cache" and "normal load", I guess.

ccing ben, who did this originally and presumably had good reasons for bypassing
cache completely.

Comment 10

[Blake Ross]

I did this originally, not Ben, and I don't recall having any good reason :-)

Comment 11

[Darin Fisher]

cc'ing pavlov, cuz i believe he added logic to imagelib to prevent shift-reload
(LOAD_BYPASS_CACHE) from fetching the same image over-and-over again when it
appears multiple times in a page.

Comment 12

[Samir Gehani]

Nav triage team: nsbeta1-

Comment 13

[sairuh (rarely reading bugmail)]

renominating --i didn't see, or perhaps missed the supposed discussion for this
in nav triage. (i had been on vacation.)

samir or paul, pls discuss this with me offline as to why it was minused.

Comment 14

[Peter Trudelle]

nsbeta1- per Nav triage team. This is an extremely advanced usage (very few
people even know about it), for which there is a workaround.  This isn't really
dataloss either, since the data is still intact on the server.

Comment 15

[Jesse Ruderman]

Bug 120809, "Save as function refetches data or images that are in the cache",
seems to think it's the menu item that should be changed, not shift+click. 
Using if-modified-since (for both, of course) would be a reasonable compromise.

See also bug 66723, "Download window should not appear when saving from cache".
 If this bug is fixed using if-modified-since, it will be possible to fix 66723
for "save image as" but not for "save link as".

Comment 16

[sairuh (rarely reading bugmail)]

nominating for buffy --looks like this affects embedded apps as well, like chimera.

Comment 17

[Samir Gehani]

Nav triage team: nsbeta1+/adt3

Comment 18

[Boris Zbarsky [:bzbarsky]]

So what is the consensus?  That we should be using LOAD_NORMAL or
LOAD_FROM_CACHE in this code and never bypassing cache explicitly?  Blake?  Your
call.

Comment 19

[Samir Gehani]

Nav triage team: nsbeta1-

Comment 20

[Boris Zbarsky [:bzbarsky]]

Adam, this is the bug about what flags "save link as" should pass that I
mentioned to you sometime in the near past...

Comment 21

[OstGote!]

Please change the summary to add "Shift+click ..." because Option+click is a
Mac-specific term and this bug is not Mac-only. 

Comment 22

[Asa Dotzler [:asa]]

*** Bug 227621 has been marked as a duplicate of this bug. ***