Skip to content

Conversation

@AdviewOpen
Copy link

we found some ads contents with mraid may remove EventListener in listeners function. for example:
function addVchange(){
mraid.isViewable()?viewChangeCB(true):mraid.addEventListener("viewableChange",viewCB);//add cb
mraid.addEventListener("viewableChange",viewCB2);//add cb2
}
function viewCB(n){
console.log('+++++++ viewCB() ++++++++');
mraid.removeEventListener("viewableChange", viewCB));//remove itself
}
function viewCB2(n){
console.log('+++++++ viewCB2() ++++++++');
}
so, if use currently mraid.js, in viewCB(), listener itself will be removed and next item will be null, that mean viewCB2() will not be triggered, we already found some ads contains like this usage, so we change the mraid.fireEvent(), we make a copy when events needs to be fired, if listener function remove itself, the fireEvent()'s array will not be cracked and keep its original order, then all of EventListners will be triggered correctly.
the same issue we have committed pull request on ios-sdk #942. but prebid-android-sdk seems use remote url to download mraid.js into PBM library ,so we changed mraid.js in scripts\js, pls review it.
greate thx.

@YuriyVelichkoPI
Copy link
Contributor

@ValentinPostindustria , please take a look on this PR

@ValentinPostindustria
Copy link
Collaborator

Hello.
Clarification for the issue. If we isolate the current logic of the handlers (in mraid.js), it looks like this:

let handlers = [];

handlers.push(() => console.log("handler1"));
handlers.push(() => {
    console.log("handler2");
    // Same method as mraid.js use
    handlers.splice(1, 1);
});
handlers.push(() => console.log("handler3"));


// Calling handlers
for (var i = 0; i < handlers.length; i++) {
    handlers[i]();
}

Proposed solution logic like this:

let handlers = [];

handlers.push(() => console.log("handler1"));
handlers.push(() => {
    console.log("handler2");
    // Same method as mraid.js use
    handlers.splice(1, 1);
});
handlers.push(() => console.log("handler3"));


// Calling handlers
let copyHandlers = [];
for (const handler in handlers) {
    copyHandlers[handler] = handlers[handler];
}
for (var i = 0; i < copyHandlers.length; i++) {
    copyHandlers[i]();
}

In the PR on iOS, @YuriyVelichkoPI said there is a risk of calling the same listeners twice. Can you describe the reasons for double calls? fireEvent() is an internal function, so calling it twice seems like an error.

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.

4 participants