-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Onetag update #5503
Onetag update #5503
Conversation
This reverts commit f1b0ede
Link to the docs PR: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi there, please see my comment regarding window.localStorage. Also, tomorrow will be my last day before going on vacation, so if you want this approved in time, try to fix it before tomorrow evening.
modules/onetagBidAdapter.js
Outdated
} | ||
const payloadString = JSON.stringify(payload); | ||
try { | ||
if (window.localStorage) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prebid offers a storageManager
class to deal with localStorage or cookies. check out, for example, bidder widespaceBidAdapter (https://github.com/prebid/Prebid.js/blob/master/modules/widespaceBidAdapter.js#L9) and how they use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @musikele , we just pushed the requested changes. Hope you have time enough to review them before your holidays :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Type of change
Description of change
Fixes some bugs. Adds field 'version' to requests.