713052 - disable alt+click or alt+enter to save links by default (with a hidden preference to re-enable)

Status: RESOLVED FIXED

(Firefox :: Keyboard Navigation, defect)

Description

[Johan C]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a2) Gecko/20111220 Firefox/10.0a2
Build ID: 20111220042029

Steps to reproduce:

Clicked a link too soon after alt-tabbing. / accidentally activated ALT just before or while clicking a link.


Actual results:

The link was saved (downloaded).


Expected results:

The link should have opened.

Comment 1

[Johan C]

Attachment: patch 0.1

initial patch

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 584346 [details] [diff] [review]
patch 0.1

I'd reverse the meaning of the pref and rename it "browser.ignoreLinkAltClick" or some such. But this pref isn't very valuable as a hidden pref. If we think accidental alt+clicks are a common user annoyance, we should be addressing the problem in a way that will benefit users who have no idea what about:config is.

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to Gavin Sharp (use [e-mail] for email) from comment #2)
> as a hidden pref. If we think accidental alt+clicks are a common user
> annoyance, we should be addressing the problem in a way that will benefit
> users who have no idea what about:config is.

Sure, I think that implies that we want some UX team input on this (adding the appropriate semaphores) issue, at which point we're just haggling over the default value for the preference. I don't think we want to add more options into the already laden Tabs pane, but I'd be down with off-by-default and making it something the more technically inclined can re-enable.

Comment 4

[Alex Limi (:limi) — Firefox UX Team]

(In reply to Gavin Sharp (use [e-mail] for email) from comment #2)
> If we think accidental alt+clicks are a common user
> annoyance, we should be addressing the problem in a way that will benefit
> users who have no idea what about:config is.

Agreed. I think alt-clicking to download is something that shouldn't be the default, as it's not very obvious to people that it will download what is clicked. Sounds like changing the default behavior and introducing a pref for the people that rely on this is the way to go. (…and even possibly making the world's simplest extension to flip that pref :)

Comment 5

[Johan C]

Attachment: patch 0.3

+ reversed the meaning of the preference.
+ alt-clicking to download is no longer the default.

I have attached a bootstrapped extension which toggles the preference, and also resets it on disable/uninstall.

Comment 6

[Johan C]

Attachment: extension that toggles the pref

Comment 7

[Johan C]

Attachment: patch 0.4

+ reverse reversed the meaning

Comment 8

[:Gavin Sharp [email: [e-mail]]]

This will need some additional test fixes before it can land (spoke to Johan on IRC). I've also pushed this to try to weed out any other potential failures.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5c84280687df

Also, to make life easier for the people checking your patches in for you, please follow the below directions for future patches you submit. Thanks!
https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Wow, awesome midair collision. Backing out...

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Backed out.
https://hg.mozilla.org/integration/mozilla-inbound/rev/d71e85f35751

Comment 12

[Mozilla RelEng Bot]

Try run for 6a25d9512584 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=6a25d9512584
Results (out of 215 total builds):
    success: 166
    warnings: 47
    failure: 1
    other: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-6a25d9512584

Comment 13

[:Gavin Sharp [email: [e-mail]]]

So based on the try results, it looks like you have two tests that need updating:
browser/base/content/test/browser_contentAreaClick.js
browser/base/content/test/browser_locationBarCommand.js

The easiest thing to do is probably to just have those tests set the pref to true temporarily, similar to how browser/base/content/test/browser_urlbarAutoFillTrimURLs.js sets browser.urlbar.trimURLs.

Comment 14

[Johan C]

Attachment: patch 0.5

(In reply to Ryan VanderMeulen from comment #9)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/5c84280687df
> 
> Also, to make life easier for the people checking your patches in for you,
> please follow the below directions for future patches you submit. Thanks!
> https://developer.mozilla.org/en/
> Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-
> in_for_me.3F

Apologies, fixed.

Updated the following tests:
browser/base/content/test/browser_contentAreaClick.js
browser/base/content/test/browser_locationBarCommand.js

Comment 15

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 601727 [details] [diff] [review]
patch 0.5

Looks good, but you don't need the try/catch around the calls to clearUserPref in browser_contentAreaClick.js. r=me with those removed.

Comment 16

[Johan C]

Attachment: patch 0.5.1

+removed all try/catch

Comment 17

[Mozilla RelEng Bot]

Autoland Patchset:
	Patches: 601735
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/pushloghtml?changeset=6d2a44f16bcd
Try run started, revision 6d2a44f16bcd. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=6d2a44f16bcd

Comment 18

[Mozilla RelEng Bot]

Try run for 6d2a44f16bcd is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=6d2a44f16bcd
Results (out of 219 total builds):
    exception: 2
    success: 175
    warnings: 40
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-6d2a44f16bcd

Comment 19

[Ryan VanderMeulen [:RyanVM]]

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

Comment 20

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

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

Comment 21

[Michal Čaplygin [:myf]]

Just quick summary of current state (for random googlers):
- config pref key is "browser.altClickSave",
- off by default, so no action needed after upgrade to make it work,
- planned for Fx 13,
- already present in Nightlies.

Comment 22

[:Gavin Sharp [email: [e-mail]]]

(In reply to Michal Čaplygin [:myf] from comment #21)
> Just quick summary of current state (for random googlers):
> - config pref key is "browser.altClickSave",
> - off by default, so no action needed after upgrade to make it work,

It's not clear what "it" you mean in "make it work". To be clear: alt+click/alt+enter to save was disabled by default by this patch, and users who wish to keep that functionality need to manually adjust the (hidden) preference.

Comment 23

[Reuben Morais [:reuben]]

Beta merges in 11 days and and user docs are still needed! Most imageboard users use this feature all the time, so it might be worth mentioning this in the release notes.

Comment 24

[trebnoj]

Please restore the 'alt click to save' feature by default in the next version of firefox. The justification for removing this was pretty weak, and you are doing nothing but making it harder for casual users of firefox to download files. Advanced users who want to remove this feature can do so from the config page, but there really is no reason to remove this feature for the rest of us. If this feature is not restored by FF14, I will officially be recommending chrome to my customers over firefox, as they still retain this feature.

Comment 25

[Roman]

The justification for removing this makes very little sense indeed. It applies equally well to why Ctrl+Click should also be removed.

Can this default be reconsidered? I could open a separate bug for that.

Comment 26

[Timwi]

It seems to me that the fix completely missed the problem. The underlying problem has not been addressed and it still there.

The underlying problem is that a single click, with no Ctrl or Alt pressed, can be misinterpreted by Firefox as Alt+Click or Ctrl+Click when it is running slow/under stress/lagging. Firefox should not misinterpret input in this way. Should I open a new bug for this and describe the underlying problem in more detail?

Comment 27

[Nick]

I agree with the others that this change was poorly thought-out and incredibly weakly justified.  It should be restored to the previous functionality with a "disable altClick download" pref.

As for the argument that holding a MODIFIER key will MODIFY the click behavior -- that's absurd.  How that pitiful reason resulted in the *removal* of a longstanding and often-used feature of the browser is incredible.

Please reconsider this shortsighted action.

Comment 28

[Roman]

Filed bug 876546.

Comment 29

[Dave Webber]

I have seen countless sites, and overheard dozens of intro computing classes (in several schools and training centres), where alt+click-to-download was explicitly directed. I’ve even taught the feature when volunteering in such classes. Now, non-technical persons, who are typically confused, frustrated, or scared by anything to do with <about:config>, ask teachers, trainers, tutors, and support technicians why alt+click-to-download is broken. Are you sure this was the best resolution? 

(As an aside, I would suggest that the only features which should be hidden behind <about:config> are those which are only expected to be sought by the sort of users who understand <about:config>.) 

As an analogy to the alternative solution I’ll suggest below, consider this difference: If, while using another program (perhaps a game with “WASD” navigation), I hold a letter-key, say “w”, and then click in this very textarea in Fx: then, no “w”s are inserted here. The fact that I am holding "w" while focused on this textarea is irrelevant to the textarea, because the “w”-keydown preceded, rather than following, the coming into foreground of the window (in which this textarea is in focus). However, if I (first) focus on div#add_comment, which is ancestral to this textarea, (second) press-and-hold 
“w”, and third focus this textarea, then a run of “w”s begins being inserted at the insertion point.

If a common mode of user error is to accidentally keep alt held down while clicking a link, after using alt+tab to switch windows, then perhaps alt+click-to-download could be suppressed when the window did not receive the keydown event which led to alt being held during the click event. In that case, when the window is brought into foreground via alt+tab, since the window was in background before tab-keydown, and alt-keydown necessarily preceded tab-keydown, the window will not have received the alt-keydown; thus, alt’s depressed state can be taken as intended for the OS’s window manager, and considered irrelevant to the link’s interpretation of the click. 

I believe this would resolve the conflict between the two concerns, without burying a feature used by non-technical users in a place where non-technical users are wary to go.