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

New mobfox prebid adapter #5978

Merged
merged 4 commits into from
Nov 30, 2020
Merged

New mobfox prebid adapter #5978

merged 4 commits into from
Nov 30, 2020

Conversation

mobfxoHB
Copy link
Contributor

Type of change

  • New bidder adapter

Description of change

New mobfox adapter

  • test parameters for validating bids
                {
                    code:'1',
                    mediaTypes:{
                        banner: {
                            sizes: [[300, 250]],
                        }
                    },
                    bids:[
                        {
                            bidder: 'mobfoxpb',
                            params: {
                                placementId: 0
                            }
                        }
                    ]
                },
                {
                    code:'1',
                    mediaTypes:{
                        video: {
                            playerSize: [640, 480],
                            context: 'instream'
                        }
                    },
                    bids:[
                        {
                            bidder: 'mobfoxpb',
                            params: {
                                placementId: 0
                            }
                        }
                    ]
                },
                {
                    code:'1',
                    mediaTypes:{
                        native: {
                            title: {
                                required: true
                            },
                            icon: {
                                required: true,
                                size: [64, 64]
                            }
                        }
                    },
                    bids:[
                        {
                            bidder: 'mobfoxpb',
                            params: {
                                placementId: 0
                            }
                        }
                    ]
                }
            ];

Be sure to test the integration with your adserver using the Hello World sample page.

Copy link
Collaborator

@Rothalack Rothalack left a comment

Choose a reason for hiding this comment

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

I was able to confirm your Banner set up was working. I was unable to confirm for Native and Video. This may just be a result of my set up. This is my first review and I'm not fully set up yet.

I saw that this may need a merge from master, seeing 22 commits behind.

I've been told that circleci often just needs to be reran, I do not have proper access yet to see the logs from circleci.

@harpere I would like to request a second review to ensure as I have not been fully trained on the system yet.

@mobfxoHB
Copy link
Contributor Author

@Rothalack Good day to you, my congratulations on your first review. Here is basic HTML page to test setup - it will result output to console containig responses for every mediatype.
`


<script src="Prebid.js/build/dist/prebid.js"></script>


<script>
var PREBID_TIMEOUT = 2000;
var FAILSAFE_TIMEOUT = 3000;

        var adUnits = [
            {
                code:'1',
                mediaTypes:{
                    banner: {
                        sizes: [[300, 250]],
                    }
                },
                bids:[
                    {
                        bidder: 'mobfoxpb',
                        params: {
                            placementId: 0
                        }
                    }
                ]
            },
            {
                code:'1',
                mediaTypes:{
                    video: {
                        playerSize: [640, 480],
                        context: 'instream'
                    }
                },
                bids:[
                    {
                        bidder: 'mobfoxpb',
                        params: {
                            placementId: 0
                        }
                    }
                ]
            },
            {
                code:'1',
                mediaTypes:{
                    native: {
                        title: {
                            required: true
                        },
                        icon: {
                            required: true,
                            size: [64, 64]
                        }
                    }
                },
                bids:[
                    {
                        bidder: 'mobfoxpb',
                        params: {
                            placementId: 0
                        }
                    }
                ]
            }
        ];

        var pbjs = pbjs || {};
        pbjs.que = pbjs.que || [];

        const cb = (a,b) => {
            console.log(a,b)
        }

        
        pbjs.que.push(function() {
            pbjs.addAdUnits(adUnits);
            pbjs.requestBids({
                bidsBackHandler: cb,
                timeout: PREBID_TIMEOUT
            });
        });
    </script>
</body>
` To make it work - create index.html, put this code in file and clone Prebid.js repo in the same folder, then as usual - npm install , gulp build and open newly created page Hope it will help you a little

@Rothalack
Copy link
Collaborator

I've gotten access to Circleci and I see it is a failure to test IE 11. It seems like every PR is failing on IE 11 and/or Safari, so I don't know if that has been addressed yet.

I've now confirmed your banner, video and native bids are working in the regular test and gdpr test.

This should still need a merge with master, showing 22 commits behind.

@mobfxoHB
Copy link
Contributor Author

@Rothalack I've merged masters with upstream, but really it is not necessary for PR to be on the same commit as main repository. Commits would not be reverted. Simple example - while you reviewing this PR - new commits are arriving to main repository , making current PR always stay behind

@Rothalack
Copy link
Collaborator

On line 37 in the main adapter, the URL function you are using isn't supported by IE, which is what is not passing in CircleCI
https://developer.mozilla.org/en-US/docs/Web/API/URL/URL

In the same block of code, you're accessing window and window.top which can be accessed rather by utils.getWindowTop and utils.getWindowLocation, etc. If you need host/pathname, it's probably best to just use utils.getWindowTop and use the result window object to get location, etc. You can code defensively for if these return empty.

Credit of this feedback to Robert

@Rothalack
Copy link
Collaborator

LGTM

@Rothalack Rothalack marked this pull request as draft November 30, 2020 16:07
@Rothalack Rothalack marked this pull request as ready for review November 30, 2020 17:10
@Rothalack Rothalack merged commit 18086e4 into prebid:master Nov 30, 2020
stsepelin pushed a commit to cointraffic/Prebid.js that referenced this pull request May 28, 2021
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