Skip to content

Do not use functions as keys in timeouts hash#22

Merged
Fetz merged 2 commits intoMailOnline:masterfrom
diggabyte:fix-clear-timeouts
Oct 26, 2016
Merged

Do not use functions as keys in timeouts hash#22
Fetz merged 2 commits intoMailOnline:masterfrom
diggabyte:fix-clear-timeouts

Conversation

@diggabyte
Copy link
Copy Markdown
Contributor

@diggabyte diggabyte commented Oct 25, 2016

The timeouts object is using callback functions as the keys in the hash. The javascript interpretor invokes .toString() on the callback function because objects cannot be used as keys.

This means you end up with a key like so:

"function () {
        // TODO avoid leaking arguments
        // https://github.com/petkaantonov/bluebird/wiki/Optimization-killers#32-leaking-arguments
        if (onSuccess.apply(this, arguments)) {
          clearTimeout(timeoutID);
        }
    }"

The problem is that this is NOT a unique key, which can causes key collisions in the timeouts hash.

My proposed fix is to directly clear the timeout ID (which is unique) upon success.
Tests are passing for this change set.

On a side-note, it seems that the bin output does not match what is checked into master. I've updated those with this pull request as well.

@Fetz Fetz merged commit 74908bf into MailOnline:master Oct 26, 2016
@Fetz
Copy link
Copy Markdown
Contributor

Fetz commented Oct 26, 2016

👍

@diggabyte diggabyte deleted the fix-clear-timeouts branch October 27, 2016 18:50
iq-dot added a commit to iq-dot/VPAIDHTML5Client that referenced this pull request Jul 28, 2017
* Added a system to configure on a per client basis any page selectors our player can hook onto
* Added telegraph article body detection and attaching to it
* Add telegraph selector detection.
* Separate client configuration into a separate file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants