Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Page MenuHomePhabricator

Use delegation to reduce number of event listeners on history pages
Closed, ResolvedPublic

Description

Notice this line in /resources/src/mediawiki.action/mediawiki.action.history.js:

$lis.find( 'input[name="diff"], input[name="oldid"]' ).on( 'click', updateDiffRadios );

It binds two event listeners for every history line. It isn't scalable and can get huge.
For perspective: a default 50-line page will already have 100 listeners, displaying 500 lines will bind 1000 listeners.… (and I even have a custom script to display 5000 lines at once, that binds 10000 listeners!)

Instead, we could take advantage of jQuery's delegated event handlers, so that only one event listener is bound (to the history container), independently from the number of lines.

Event Timeline

Change 842495 had a related patch set uploaded (by Gerrit Patch Uploader; author: Francois Pignon):

[mediawiki/core@master] Use delegation to reduce number of event listeners on history pages

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

Refs some related commits:

The last one is the most important. Notably, instead of having a single ul.mw-contributions-list container, the lines are now split into separate ul.mw-contributions-list containers, for each different day.

I'm taking the opportunity to change the event type from "click" to the more modern "change", supported by IE 9+ (refs caniuse, MDN). See patchset 2.

Also, in the present use case, it reduces the amount of triggered events.

Change 842495 merged by jenkins-bot:

[mediawiki/core@master] Use delegation to reduce number of event listeners on history pages

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

matmarex assigned this task to Od1n.
matmarex subscribed.

Thanks for the patch!

Some notes about the implementation:

$pagehistory = $( '#pagehistory' ),
$lis = $pagehistory.find( '.mw-contributions-list > li' );

I kept the '.mw-contributions-list > li' and did not shorten to 'li', although it would have been more performant.
Because any false positive (think of local script, or future change in the interface) would cause issues. And as so, the selector is 100% equivalent to the previous one.

$pagehistory.on( 'change', 'input[name="diff"], input[name="oldid"]', updateDiffRadios );

I used simply 'input[name="diff"], input[name="oldid"]' instead of for example '.mw-contributions-list > li input[name="diff"], .mw-contributions-list > li input[name="oldid"]',
because I think it's unlikely we could get a false positive by encountering an unrelated input with the same name. Also, any false positive wouldn't harm much, it would juste execute updateDiffRadios() uselessly.