You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
sebiniemann
changed the title
Removed the extra argument from removeEventListener and merges eventListenerOptions into addEventListener
Removes the extra argument from removeEventListener and merges eventListenerOptions into addEventListener
Feb 7, 2019
sebiniemann
changed the title
Removes the extra argument from removeEventListener and merges eventListenerOptions into addEventListener
Removes the extra argument from removeEventListener
Feb 7, 2019
The line to impact to address #5303 would be the call to removeResizeListener, but this has already been fixed in #5970. And according the MDN, removeEventListener takes 3 arguments, the last one recommended to be the same as the one used in addEventListener:
It's worth noting that some browser releases have been inconsistent on this, and unless you have specific reasons otherwise, it's probably wise to use the same values used for the call to addEventListener() when calling removeEventListener()
I'm not sure we should have altered the code just if a analyzer says so. Does SonarQube know every implementation of removeEventListener that's in the wild (or even those we are targeting)? I'd trust MDN on this.
@kurkle SonarQube didn't report an issue with removeEventListener but with the call of our own removeResizeListener method, which indeed doesn't accept 2 arguments.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5303