Fix "Unsubscribe" button on accounts page in Firefox. #32
Conversation
Or a less-sensational title that explains the rest of the changeset: avoid using the global "event" and avoid using "event" as a variable name. Some browser environments define a global "event" which is set to the current event that is being dispatched for the duration of event dispatch. Other browsers do not. As a result, since a variable named "event" may or may not shadow the global event object, it's best to avoid the whole confusion and never name a variable "event" or use a reference named "event". [1] [1] - http://eslint.org/docs/rules/no-restricted-globals
| template._showPrompt.set(true); | ||
| }, | ||
|
|
||
| "click .unsubscribe": function () { | ||
| event.preventDefault(); |
There was a problem hiding this comment.
To be clear, this access of the global is what broke under Firefox.
|
I actually think that avoiding naming the parameter "event" makes it more likely that someone will accidentally depend on the global by incorrectly typing "event" on one particular line but magically getting the same object as is bound to "ev" -- except on Firefox. |
|
Personally, I disagree. If we have a general "don't use As a bonus, your editor's syntax highlighter should highlight Some day we'll have ESLint (once we clean up the codebase enough to not generate thousands of warnings) and can have a rule which disallows accessing the global, in which case the rule will be able to catch bad accesses either way, but until then, I think the fact that this bug existed speaks to the wisdom of shadowing variables like this. |
|
Since we closed sandstorm-io/sandstorm#2394 I suppose we should skip the renames here. I pushed a commit that just fixes the bug itself. |
Or a less-sensational title that explains the rest of the changeset: avoid
using the global "event" and avoid using "event" as a variable name.
Some browser environments define a global "event" which is set to the current
event that is being dispatched for the duration of event dispatch. Other
browsers do not.
As a result, since a variable named "event" may or may not shadow the global
event object, it's best to avoid the whole confusion and never name a variable
"event" or use a reference named "event". [1]
[1] - http://eslint.org/docs/rules/no-restricted-globals