Skip to content

Event: Remove the event.which shim #4765

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
mgol merged 1 commit into from
Aug 26, 2020
Merged

Event: Remove the event.which shim #4765

mgol merged 1 commit into from
Aug 26, 2020

Conversation

@mgol
Copy link
Member

@mgol mgol commented Jul 29, 2020 •
edited
Loading

Summary

All supported browsers implement this property by themselves. The shim was only
needed for IE <9.

Fixes gh-3235
Ref gh-4755

-101 bytes!

Checklist

@mgol mgol self-assigned this Jul 29, 2020
@mgol mgol mentioned this pull request Jul 29, 2020
Closed
@mgol
Copy link
Member Author

mgol commented Jul 29, 2020

Since this was only needed for IE <9, perhaps we could land it in 3.x-stable as well?

@mgol mgol marked this pull request as ready for review July 29, 2020 15:48
@mgol
Event: Remove the event.which shim
a684919 
All supported browsers implement this property by themselves. The shim was only
needed for IE <9.

Fixes jquerygh-3235
Ref jquerygh-4755
@mgol mgol force-pushed the event-which-shim-gone branch from fc1ac55 to a684919 Compare July 29, 2020 17:07
@mgol mgol mentioned this pull request Jul 29, 2020
Closed
@Krinkle
Copy link
Member

Krinkle commented Aug 17, 2020

Code LGTM, but:

@dmethvin wrote at #3235:

It's a bit tricky to test because it needs a real event and not a simulated one.

Have we tested this on a standalone page in IE9-11 to confirm once more that it sets e.which correctly for key events and mouse buttons?

@dmethvin
Copy link
Member

dmethvin commented Aug 17, 2020

If it sets e.which at all, that value is used. Since IE was the source of the property it's unlikely they have taken it out in the meantime so I think we're safe.

@mgol mgol added the Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label Aug 21, 2020
@timmywil timmywil removed the Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label Aug 24, 2020
timmywil
timmywil approved these changes Aug 24, 2020
View reviewed changes
Copy link
Member

@timmywil timmywil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't argue with Dave

@mgol mgol removed the Needs review label Aug 25, 2020
@mgol mgol added this to the 3.6.0 milestone Aug 25, 2020
@mgol mgol merged commit 1a5fff4 into jquery:master Aug 26, 2020
@mgol mgol deleted the event-which-shim-gone branch August 26, 2020 12:10
mgol added a commit that referenced this pull request Aug 26, 2020
@mgol
Event: Remove the event.which shim
2bf9793 
All supported browsers implement this property by themselves. The shim was only
needed for IE <9.

Fixes gh-3235
Closes gh-4765
Ref gh-4755

(cherry picked from commit 1a5fff4)
@mgol
Copy link
Member Author

mgol commented Aug 26, 2020

Landed on master in 1a5fff4 & on 3.x-stable in 2bf9793.

@zrpnr zrpnr mentioned this pull request May 12, 2021
Closed
@mgol mgol mentioned this pull request Nov 24, 2022
Closed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@dmethvin dmethvin dmethvin approved these changes

@timmywil timmywil timmywil approved these changes

Assignees

@mgol mgol

Labels

Event

Milestone

3.6.0

Development

Successfully merging this pull request may close these issues.

Remove event.which property fill

4 participants

@mgol @Krinkle @dmethvin @timmywil