Page MenuHomePhabricator
Create Task
Maniphest T277407

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

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.

Details

SubjectRepoBranchLines +/-
Replace occurances of add/removeEvent with add/removeEventListenerwikimedia/portalsmaster+23 -49
Replace::occurances of outdated codewikimedia/portalsmaster+4 -32
Customize query in gerrit

Related Objects
Search...

Event Timeline

Jdrewniak created this task.Mar 14 2021, 2:27 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 14 2021, 2:27 PM
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.
Jdrewniak added a parent task: T277404: [EPIC] Modernize portals code for browser support and maintainability.
Jdrewniak mentioned this in T273179: Update the front-page of Wikimedia projects.Mar 14 2021, 2:38 PM
Palak199 claimed this task.Mar 14 2021, 4:03 PM
Palak199 subscribed.

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

veeru153 subscribed.EditedMar 14 2021, 4:04 PM

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.

Palak199 added a comment.Mar 14 2021, 5:42 PM

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.

Tsiruot subscribed.Mar 15 2021, 1:27 AM

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

Commit_message_guidelines

I'd recommend going through these :)

gerritbot added a comment.Mar 15 2021, 4:17 PM

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

gerritbot added a project: Patch-For-Review.Mar 15 2021, 4:18 PM

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

Palak199 added a comment.Mar 15 2021, 4:24 PM
In T277407#6912388, @Tsiruot wrote:

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?

Tsiruot added a comment.EditedMar 15 2021, 4:26 PM

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

Imacchiavello subscribed.Mar 15 2021, 7:59 PM
Palak199 added a comment.Mar 16 2021, 3:34 PM

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 ) );
Jdrewniak added a comment.Mar 16 2021, 9:16 PM

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 ) );
gerritbot added a comment.Mar 17 2021, 1:34 AM

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

Palak199 added a comment.Mar 17 2021, 1:37 AM

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

Aklapper added a comment.Mar 17 2021, 6:55 AM

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

Palak199 added a comment.Mar 17 2021, 7:07 AM
In T277407#6920374, @Aklapper wrote:

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

gerritbot added a comment.Mar 21 2021, 11:32 AM

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

gerritbot added a comment.Mar 21 2021, 11:32 AM

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

gerritbot added a comment.Mar 21 2021, 11:33 AM

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

Maintenance_bot removed a project: Patch-For-Review.Mar 21 2021, 12:10 PM
gerritbot added a comment.Mar 21 2021, 7:12 PM

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

gerritbot added a project: Patch-For-Review.Mar 21 2021, 7:12 PM
gerritbot added a comment.Mar 21 2021, 7:50 PM

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

Krinkle awarded a token.Mar 26 2021, 5:22 AM
Helix17 subscribed.Mar 29 2021, 7:40 AM

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

Aklapper added a comment.Mar 29 2021, 8:07 AM

@Helix17: Hi, please see previous comments in this task and read https://www.mediawiki.org/wiki/New_Developers#Communication - thanks.

Palak199 added a comment.Mar 29 2021, 8:20 AM

@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 added a comment.Mar 29 2021, 8:37 AM

@Palak199: Hi, please see https://www.mediawiki.org/wiki/New_Developers#Communication and bring up issues with Gerrit in Zulip - thanks a lot!

Helix17 added a comment.Mar 29 2021, 1:01 PM

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

gerritbot added a comment.Mar 31 2021, 9:16 AM

Change 673808 merged by jenkins-bot:

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

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

Bharatkhatri351 closed this task as Resolved.Apr 1 2021, 12:17 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL