Skip to content
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

Appnexus native view tracker #4022

Merged
merged 19 commits into from
Jul 30, 2019
Merged

Appnexus native view tracker #4022

merged 19 commits into from
Jul 30, 2019

Conversation

jsnellbaker
Copy link
Collaborator

Type of change

  • Feature

Description of change

Adds some logic to incorporate the appnexus view-ability script for native bids.

As the script requires some information about the bid that's generated after it runs through the auction, there is some custom logic that will replace some place-holders values in the script before it executes on the page. This logic is triggered via the onBidWon adapter function.

@mkendall07
Copy link
Member

This pull request introduces 1 alert when merging e04275d into c2bdb94 - view on LGTM.com

new alerts:

  • 1 for Incomplete regular expression for hostnames

This project has automated code review enabled, but doesn't use the LGTM GitHub App. Migrate over by installing the app. Read about the benefits of migrating to GitHub Apps in the blog.


Comment posted by LGTM.com

@mkendall07
Copy link
Member

This pull request introduces 1 alert when merging 4057c13 into c2bdb94 - view on LGTM.com

new alerts:

  • 1 for Incomplete regular expression for hostnames

This project has automated code review enabled, but doesn't use the LGTM GitHub App. Migrate over by installing the app. Read about the benefits of migrating to GitHub Apps in the blog.


Comment posted by LGTM.com

Copy link
Member

@mkendall07 mkendall07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM. Seems like a lot of code to add to the adapter, any chance we can host this code / download it only for native responses?

@jsnellbaker jsnellbaker merged commit e603156 into master Jul 30, 2019
VideoReach pushed a commit to VideoReach/Prebid.js that referenced this pull request Aug 1, 2019
* First pass: adding viewability script to rtbBid

* Added viewability scrit in the right place

* Clean up

* Corrected script position, added a workaround to get the adId after the bid's creation and fixed tests

* Removed debugging logs

* More cleanup

* Opted to append rtbBid.viewability.config to the existing (nativeAd.javascript_trackers) in the bidAdapter's newBid function

* Removed AnalyticsAdaper code and added bidAdapter.onBidWon to check for our jstracker and update their src attributes.

* Added a way to differentiate scripts and clean up

* Changed newBid to disarm jstracker before it's ready and changed onBidWon to arm it. This prevents the tracker from being loaded twice.

* Making the code more robust

* Grouped code in a function. Also changed parameters to reflect that finding the element will now fall be a view_js responsability.

* Changed parameter pbjs_iframeid name to pbjs_auc so its not misleading

* Variable name change to better reflect its nature

* Fixing URL parameters (; separated and replacing completely the useless one)

* undo changes to appnexus Analytics adapter

* update regex variable

* update regex again
leonardlabat pushed a commit to criteo-forks/Prebid.js that referenced this pull request Aug 6, 2019
* First pass: adding viewability script to rtbBid

* Added viewability scrit in the right place

* Clean up

* Corrected script position, added a workaround to get the adId after the bid's creation and fixed tests

* Removed debugging logs

* More cleanup

* Opted to append rtbBid.viewability.config to the existing (nativeAd.javascript_trackers) in the bidAdapter's newBid function

* Removed AnalyticsAdaper code and added bidAdapter.onBidWon to check for our jstracker and update their src attributes.

* Added a way to differentiate scripts and clean up

* Changed newBid to disarm jstracker before it's ready and changed onBidWon to arm it. This prevents the tracker from being loaded twice.

* Making the code more robust

* Grouped code in a function. Also changed parameters to reflect that finding the element will now fall be a view_js responsability.

* Changed parameter pbjs_iframeid name to pbjs_auc so its not misleading

* Variable name change to better reflect its nature

* Fixing URL parameters (; separated and replacing completely the useless one)

* undo changes to appnexus Analytics adapter

* update regex variable

* update regex again
sa1omon pushed a commit to gamoshi/Prebid.js that referenced this pull request Nov 28, 2019
* First pass: adding viewability script to rtbBid

* Added viewability scrit in the right place

* Clean up

* Corrected script position, added a workaround to get the adId after the bid's creation and fixed tests

* Removed debugging logs

* More cleanup

* Opted to append rtbBid.viewability.config to the existing (nativeAd.javascript_trackers) in the bidAdapter's newBid function

* Removed AnalyticsAdaper code and added bidAdapter.onBidWon to check for our jstracker and update their src attributes.

* Added a way to differentiate scripts and clean up

* Changed newBid to disarm jstracker before it's ready and changed onBidWon to arm it. This prevents the tracker from being loaded twice.

* Making the code more robust

* Grouped code in a function. Also changed parameters to reflect that finding the element will now fall be a view_js responsability.

* Changed parameter pbjs_iframeid name to pbjs_auc so its not misleading

* Variable name change to better reflect its nature

* Fixing URL parameters (; separated and replacing completely the useless one)

* undo changes to appnexus Analytics adapter

* update regex variable

* update regex again
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants