Page MenuHomePhabricator

Replace add/removeEvent methods with add/removeEventListener
Closed, ResolvedPublic

Description

In the polyfill.js file, there exists a window.attachedEvents and two wrapper methods addEvent and removeEvent.

These methods were created to work around limitations of the once popular Internet Explorer browser to accommodate it’s attachEvent and removeEvent methods. Since IE9 (the oldest version that can still access Wikimedia websites) supports addEventListener and removeEventListener, these global methods are no longer necessary.

Acceptance Criteria

The work here involves deleting the following methods from the polyfills file:

  • window.attachedEvents
  • addEvent
  • removeEvent

And replacing all occurrences of addEvent and removeEvent from the codebase with native addEventListener and removeEventListener methods.

Event Timeline

Jdrewniak triaged this task as Medium priority.Mar 14 2021, 2:27 PM
Jdrewniak lowered the priority of this task from Medium to Low.
Jdrewniak moved this task from Untriaged to Code Quality / Tech Debt on the Wikimedia-Portals board.
Palak199 added a subscriber: Palak199.

I am Palak, another open source enthusiast and I would like to contribute to this project.

I'd like to take this microtask. This would help me get familiarised with the codebase. However, I am not sure if more than one person can claim a task.

https://gerrit.wikimedia.org/r/c/wikimedia/portals/+/672052
This patch removes the outdated functions. Please review and let me know if there are any other changes needed.

Hey @Palak199 , I think you missed on these guidelines.

Commit_message_guidelines

I'd recommend going through these :)

Change 672452 had a related patch set uploaded (by Palak199; owner: Palak199):
[wikimedia/portals@master] Replace::occurances of outdated code

https://gerrit.wikimedia.org/r/672452

Change 672453 had a related patch set uploaded (by Palak199; owner: Palak199):
[wikimedia/portals@master] Merge branch 'master' of ssh://gerrit.wikimedia.org:29418/wikimedia/portals into T277407

https://gerrit.wikimedia.org/r/672453

Hey @Palak199 , I think you missed on these guidelines.

Commit_message_guidelines

I'd recommend going through these :)

Thanks for reviewing, I have amended the commit message, https://gerrit.wikimedia.org/r/c/wikimedia/portals/+/672453
Does this work now?

@Palak199 I am not a mentor. I was just helping you out in writing commits the correct way.

I have made all the changes accordingly except the one below. Would you please guide me on this one? How should this one be replaced

/**
     * Attaching type-ahead query action to 'input' event.
     */
    addEvent( searchInput, inputEvent, _.debounce( function () {
        typeAhead.query( searchInput.value, document.getElementById( 'searchLanguage' ).value );
    }, 100 ) );

Hi @Palak199,
The function you mention can be rewritten in much the same way as other ones:


/**
 * Attaching type-ahead query action to 'input' event.
 */
searchInput.addEventListener(inputEvent, _.debounce( function () {
    typeAhead.query( searchInput.value, document.getElementById( 'searchLanguage' ).value );
}, 100 ) );

Change 672849 had a related patch set uploaded (by Palak199; owner: Palak199):
[wikimedia/portals@master] Merge branch 'master' of ssh://gerrit.wikimedia.org:29418/wikimedia/portals into T277407

https://gerrit.wikimedia.org/r/672849

hi @Jdrewniak,
I have made the necessary changes in all js files.
https://gerrit.wikimedia.org/r/c/wikimedia/portals/+/672848
Kindly review and let me know if this works fine now

Please don't ping in several places for patch review as it duplicates notifications from Phabricator and Gerrit - thanks.

Please don't ping in several places for patch review as it duplicates notifications from Phabricator and Gerrit - thanks.

Sure, will take care next time

Change 672849 abandoned by Jdrewniak:
[wikimedia/portals@master] Merge branch 'master' of ssh://gerrit.wikimedia.org:29418/wikimedia/portals into T277407

Reason:
I'm assuming this was uploaded by mistake. The patch with the work for T277407 is here: https://gerrit.wikimedia.org/r/c/wikimedia/portals/ /672848

https://gerrit.wikimedia.org/r/672849

Change 672452 abandoned by Jdrewniak:
[wikimedia/portals@master] Replace::occurances of outdated code

Reason:
The patch with the work for T277407 is here: https://gerrit.wikimedia.org/r/c/wikimedia/portals/ /672848

https://gerrit.wikimedia.org/r/672452

Change 672453 abandoned by Jdrewniak:
[wikimedia/portals@master] Merge branch 'master' of ssh://gerrit.wikimedia.org:29418/wikimedia/portals into T277407

Reason:
The patch with the work for T277407 is here: https://gerrit.wikimedia.org/r/c/wikimedia/portals/ /672848

https://gerrit.wikimedia.org/r/672453

Change 673808 had a related patch set uploaded (by Palak199; owner: Palak199):
[wikimedia/portals@master] Bug: T277407 Correct all the linting errors

https://gerrit.wikimedia.org/r/673808

Change 673808 had a related patch set uploaded (by Ammarpad; owner: Palak199):
[wikimedia/portals@master] Correct all the linting errors

https://gerrit.wikimedia.org/r/673808

Is this issue still open? May I work on it?

@Aklapper : Hi,
I am almost done with this task, there were some conflicts and after resolving them, I am facing some problem in sending it for review

remote: Processing changes: refs: 1
remote: Processing changes: refs: 1, done            
To ssh://gerrit.wikimedia.org:29418/wikimedia/portals.git
 ! [remote rejected]   HEAD -> refs/for/master%topic=T277407-master (the number of pushed changes in a batch exceeds the max limit 10)
error: failed to push some refs to 'ssh://gerrit.wikimedia.org:29418/wikimedia/portals.git'

Can you please guide me on it

@Aklapper I will look for other issues as @Palak199 is already working on it. Thanks

Change 673808 merged by jenkins-bot:

[wikimedia/portals@master] Replace occurances of add/removeEvent with add/removeEventListener

https://gerrit.wikimedia.org/r/673808