55686 - Option-click links doesn't save file

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P3)

Description

[Adam]

Mozilla M18 2000100608, Mac OS 9.0.4 -

1) Option-click on a link
2) The file/document is loaded in the browser

When you Option-click a link, it should be saved to disk.

Comment 1

[don]

Does option-click work that way in 4.x?

Comment 2

[Adam]

Yep. It's a way to get around the snafu of having unmapped or incorrectly mapped 

MIME types. Just option-click to download the link to disk.

Comment 3

[sairuh (rarely reading bugmail)]

[note to self: check out if alt+click does this on other platforms. or, was that
a mac-only (pp) thing in 4.x?]

Comment 4

[sairuh (rarely reading bugmail)]

yep, alt+click link will prompt use to save the link on linux and win32. marking
all/all.

i don't know how trivial or difficult this would be to implement --if easy, i
nominate for rtm. if not...helpwanted.

Comment 5

[don]

Bill, is this a regression?

Comment 6

[Bill Law]

No, it is not a regression.  This has never been possible due to the way Gecko
handles link clicks.  It decides to go to the link on its own, regardless of the
shift key state, or what Navigator might want to do.

See also bug 12056.

I don't think we should hack a fix for this for rtm.

Comment 7

[don]

OK, let's minus it then.

Comment 8

[Adam]

Note: several websites instruct users to use this method to download files to
disk. If it's not supported in Netscape 6, these websites will have to be
updated (else either the website author will receive a lot of email or there
will be a lot of bug reports about this).

Comment 9

[Gervase Markham [:gerv]]

Nominating for both relnote types, for the two facets of this problem.

Gerv

Comment 10

[Matthew T (active 1999-2002)]

Dup of bug 58841, or vice versa.

Comment 11

[sairuh (rarely reading bugmail)]

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

Comment 12

[Adam]

Reposting mpt's comments from bug 58841 (now that this behavior has been
implemented (albeit incorrectly)):

> On Mac OS, the modifier key for saving a link should be Option+click
> rather than Shift+click.

> As well as preserving 4xp, this will allow the modifier key for making
> or extending a selection to be Shift consistently whether the cursor is
> inside or outside a link or button (bug 50673).

I 100% agree with this. Option-click is the method expected by users and web
developers and should be maintained.

Comment 13

[Blake Ross]

reassigning to me

Comment 14

[Blake Ross]

Attachment: [patch] use option-click on mac

Comment 15

[Peter "jag" Annema]

Personally I'm thinking yuck... I dislike this hackish platform detection code in multiple places... Especially since there'll be more places where different platforms will want special treatment.

Comment 16

[Boris Zbarsky [:bzbarsky]]

r=bzbarsky, for what it's worth.

Comment 17

[Blake Ross]

cc'ing alecf for sr

jag, this isn't really hackish.  you just suggested replacing it with IsMac() 
or some such, but that's still doing the same 'hack' (with a cleaner name), and 
it requires pulling in a file for one tiny function.

Comment 18

[Peter "jag" Annema]

Yes, but placing it inside an isMac() function makes cleaning up the hack easier
later. Also, I don't see what your objection is to pulling in a file, assuming
the files are decently factored.

Comment 19

[Blake Ross]

...There are currently two uses of this (navigator.platform.indexOf) in all of 
LXR, three with my patch.  My objection is that, as Ben said, pulling in a file 
for a function that has the potential to be 1-2 lines long isn't worth it...for 
performance and just logical reasons.

Comment 20

[Peter "jag" Annema]

Hrm, I interpreted Ben's comment as being about factoring out code into platform
specific files/overlays.

I still say it's yucky, but if everyone else says this is the best way to do it
I'll just grmbl quietly...

Comment 21

[Alec Flett]

wait, the correct way to do this, IMO, is to add a preference to all.js that's
overridden by macprefs.js... then there is no platform-specific stuff in our
code, just our preferences.

Comment 22

[Chris McAfee]

I think we should try to get this in, nsbeta1.

Comment 23

[Marcia Knous [:marcia]]

nominating for dogfood (from sdagley's list of bugs that are good candidates for 
our next release) 

Comment 24

[Blake Ross]

Attachment: patch

Comment 25

[Alec Flett]

lookin' good!
sr=alecf

Comment 26

[Keyser Sose]

Shouldnt this be closed now since it was checked in? :)

Comment 27

[Blake Ross]

Yup. Thanks for the reminder.

Comment 28

[Simon Fraser [no longer active]]

This does not work on Mac. On Mac, option-click should save the link target to a 
file, and command-click should open it in a new window. The latter works, the 
former does not.

Comment 29

[sairuh (rarely reading bugmail)]

same observation as smfr using 2001.05.22.09 moz bits.

addendum: tested on linux, and shift+click in both 4.x and mozilla save the link 
[contrary to my 2000-10-09 14:32 comment, heh], which is expected.

Comment 30

[Bill McGonigle (not currently reading bugmail; please contact directly)]

Not clear from the comments if this was checked in or not...  It doesn't work on
Fizzilla if it was.

Comment 31

[Akkana Peck]

This (or another checkin, but this looks most likely) seems to have broken
shift-click to save on Unix.  I'll reassign that bug to blake.

Comment 32

[Blake Ross]

Umm...what checkin...?

Comment 33

[Akkana Peck]

Oh, I got the dates messed up.  You had checked in code that changed the way
this feature worked, but it was quite a while ago so it probably wasn't
responsible for this problem.  Don't know what was, then ...

Comment 34

[sairuh (rarely reading bugmail)]

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

Comment 35

[Blake Ross]

-> default owner

Comment 36

[Aaron Leventhal]

-> saari, for retriage

Comment 37

[Peter Trudelle]

nsbeta1-, per ADT triage team

Comment 38

[Simon Fraser [no longer active]]

Whaaa. Option-click is my knee-jerk reaction to download files that spew binary 
into the browser window. Without this, I have to laboriously inspect the context 
menu for the relevant item.

Comment 39

[saari (gone)]

-> sdagley for consideration. Steve, if you contest this not being nsbeta1+,
please bring it up with EDT. Simon, you could bring it up also (I suggest going
to a EDT meeting)

Comment 40

[Steve Dagley]

I agree we really want this (I hit it a _lot_) but it's not gonna make the 0.9.9 
train

Comment 41

[Blake Ross]

Attachment: does this fix it...?

Comment 42

[Steve Dagley]

Comment on attachment 70048 [details] [diff] [review]
does this fix it...?

r=sdagley (since Blake submitted the patch and it Does The Right Thing™ for me)

Comment 43

[Blake Ross]

-> me

sdagley, you tested the fix and it worked?

Comment 44

[Steve Dagley]

Blake, y, that patch works for me.  You've got my r=, you just need sr= and a=
to land it.

Comment 45

[Alec Flett]

Comment on attachment 70048 [details] [diff] [review]
does this fix it...?

Wait, are you changing Linux/Windows to match mac behavior?

is saveModifier some sort of pref?

Comment 46

[Blake Ross]

Yes:

saveModifier = pref.getBoolPref("ui.key.saveLink.shift");

So if that's set, we use shiftKey.  Otherwise, we use altKey.  The mac pref file
overrides this to false, it's true on linux/win. The cause of this bug is just
my confusion over our event system's name for mac's option key. I thought it was
metaKey (apparently), but it's altKey.

Comment 47

[Joe Hewitt (gone)]

Comment on attachment 70048 [details] [diff] [review]
does this fix it...?

sr=hewitt

Comment 48

[Mike Shaver (:shaver emeritus)]

Comment on attachment 70048 [details] [diff] [review]
does this fix it...?

a=shaver for 1.0 trunk.

Comment 49

[Mike Shaver (:shaver emeritus)]

a=shaver for the branch, too.

Comment 50

[Blake Ross]

fixed.

Comment 51

[sairuh (rarely reading bugmail)]

vrfy'd fixed using 2002.03.06.08 comm bits on mac 10.1.3. thanks a lot, blake!