-
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
dfp.buildVideoUrl bug with the url option? #2291
Comments
@terebentina |
@terebentina Can you provide a sample URL you were using where you saw the issue occur? |
Not really, nothing public yet but I can try to create a jsfiddle maybe using one of yours as a base but it probably won't happen this week. I simply noticed that when calling |
@terebentina as you may have seen with the note above, I have put together a fix for this issue. Please feel free to checkout the branch (note - the branch is for 1.x) and test it out; if you're still encountering an issue - please let me know the details so we can investigate further. |
@terebentina This should be fixed now in 1.7 release. If you continue to see any issues, please let us know. |
Type of issue
bug?
Description
I am trying to figure out if this is a bug or not. When calling
dfp.buildVideoUrl()
with theurl
option the code is splitting the url into its parts and then rebuilding it inbuildUrlFromAdserverUrlComponents()
. However, it seems to me that anycust_params
that might have been there in the initial url is getting overwritten here: https://github.com/prebid/Prebid.js/blob/master/modules/dfpAdServerVideo.js#L126-L129Which doesn't seem to be the case when calling
dfp.buildVideoUrl()
with theparams
option: https://github.com/prebid/Prebid.js/blob/master/modules/dfpAdServerVideo.js#L88-L93Platform details
This is the same in the latest 0.x and on
master
as well.The text was updated successfully, but these errors were encountered: