Skip to content

Fix issue with native ads being resized incorrectly (1.14 patch)#190

Merged
dgirardi merged 1 commit into
prebid:patch-1.14from
dgirardi:fix-rendering-resize
Nov 22, 2022
Merged

Fix issue with native ads being resized incorrectly (1.14 patch)#190
dgirardi merged 1 commit into
prebid:patch-1.14from
dgirardi:fix-rendering-resize

Conversation

@dgirardi

@dgirardi dgirardi commented Nov 8, 2022

Copy link
Copy Markdown
Collaborator

Addresses prebid/Prebid.js#9171

This test page shows incorrect rendering on latest (currently 1.14):

<html>
<head>
    <script>
        var PREBID_TIMEOUT = 6000;

        var adUnits = [
            {
                code: 'native-div',
                mediaTypes: {
                    native: {
                        adTemplate: `
                                        <div id="adunit" style="overflow: hidden; width: 335px !important;">
                                            <div>Lorem ipsum etcetera Lorem ipsum etcetera Lorem ipsum etcetera Lorem ipsum etcetera Lorem ipsum etcetera Lorem ipsum etcetera Lorem ipsum etcetera</div>
                                            <a style: 'display: block' class="clickUrl" href="##hb_native_linkurl##">
                                              <img class="image" width="400" src="##hb_native_image##" />
                                              <div class="title pb-click" pbAdId="##hb_adid##">##hb_native_title##</div>
                                              <div class="body">##hb_native_body##</div>
                                            </a>
                                        </div>
                                    `,
                        title: {
                            required: true
                        },
                        body: {
                            required: true,
                            sendId: true
                        },
                        image: {
                            required: true
                        },
                        sponsoredBy: {
                            required: true
                        },
                        icon: {
                            required: false
                        }
                    }
                },
                bids: [{
                    bidder: 'appnexus',
                    params: {
                        placementId: 13232354,
                        allowSmallerSizes: true
                    }
                }]
            }];


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

    </script>
    <!--
    <script type="text/javascript" src="http://lh.rubiconproject.com/prebid/pbjs-native2-beta.js" async></script>

    -->
    <script type="text/javascript" src="../../../build/dev/prebid.js" async></script>

    <script>
        var googletag = googletag || {};
        googletag.cmd = googletag.cmd || [];
        googletag.cmd.push(function() {
            googletag.pubads().disableInitialLoad();
        });

        pbjs.que.push(function() {
            pbjs.setConfig({debug: true});
            pbjs.setConfig({
                debugging: {
                    enabled: true,
                }
            });
            pbjs.addAdUnits(adUnits);
//        pbjs.setConfig({
//	    s2sConfig: {
//		accountId: '1001',
//		bidders: ['appnexus'],
//		endpoint: 'https://prebid-server.dev.rubiconproject.com/openrtb2/auction',
//		syncEndpoint: 'https://prebid-server.rubiconproject.com/cookie_sync',
//		adapter: 'prebidServer',
//		timeout: 1000,
//		enabled: true
//	    }
//	});
            pbjs.requestBids({
                bidsBackHandler: sendAdserverRequest
            });
        });

        function sendAdserverRequest(bids, timedOut, auctionId) {
            if (pbjs.adserverRequestSent) return;
            pbjs.adserverRequestSent = true;
            googletag.cmd.push(function() {
                pbjs.que.push(function() {
                    pbjs.setTargetingForGPTAsync();
                    googletag.pubads().refresh();
                });
            });
        }

        setTimeout(function() {
            sendAdserverRequest();
        }, PREBID_TIMEOUT);
    </script>

    <script>
        (function () {
            var gads = document.createElement('script');
            gads.async = true;
            gads.type = 'text/javascript';
            var useSSL = 'https:' == document.location.protocol;
            gads.src = (useSSL ? 'https:' : 'http:') +
                '//www.googletagservices.com/tag/js/gpt.js';
            var node = document.getElementsByTagName('script')[0];
            node.parentNode.insertBefore(gads, node);
        })();
    </script>

    <script>
        googletag.cmd.push(function() {
            googletag.defineSlot('/41758329/dgirardi-test', ['fluid'], 'native-div').setTargeting("liselect", 'issue-9131').addService(googletag.pubads());
            googletag.pubads().enableSingleRequest();
            googletag.enableServices();
        });
    </script>
</head>

<body>
<h2>Prebid Native Test</h2>

<div id='native-div'>
    <script>
        googletag.cmd.push(function() { googletag.display('native-div'); });
    </script>
</div>
</body>
</html>

@dgirardi dgirardi requested a review from musikele November 8, 2022 23:35
@musikele

musikele commented Nov 9, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

@dgirardi The test case you prepared is actually visualized better, even though I still see the height around 100px . I think the proper size should be height ~380. Can it be because of some css properties that are tricking the browser? hmm.

Here's an example of what I see, with firefox and chrome:
image
Note that this behaviour is the same of PUC v1.13 (so this PR actually brings back the old behaviour).

The only way to see the ad fully loaded is to wrap requestHeightResize in a setTimeout: It's like the browser needs some time to recalculate the body.clientHeight, and this recalculation happens async.

setTimeout(() => {
            requestHeightResize(
                bid.adId,
                document.body.clientHeight || document.body.offsetHeight,
                document.body.clientWidth
            );
        }, 300);

Values smaller than 300 will not cause the ad to fully show.

image

Of course I am not suggesting to wrap the function with a setTimeout, but I was curious to know if you see what I see and what do you think about it

@musikele

musikele commented Nov 9, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

I've done a test and if I exclude the image from the HTML the height of the creative is exactly 104; this means that since the image is loaded asynchronously it will not be added into the final height.
I've done another test where I set the height of the image to 300 (before being added to the dom) and I saw the size of the document.body.height to be set, correctly, to 404.
At this point we may clearly delegate this to our users, with a note saying that images must have a specified height otherwise the ad resize will be funky, OR we can use this other snippet that works on modern browsers:

const resizeObserver = new ResizeObserver((entries) => {
            requestHeightResize(
                bid.adId,
                entries[0].target.clientHeight || document.body.offsetHeight,
                document.body.clientWidth
            );
        });
        resizeObserver.observe(win.document.body);

Based on my tests, it hugely depends on the time needed to load the image, but in the end it will resize the ad no matter what.

@dgirardi

dgirardi commented Nov 9, 2022

Copy link
Copy Markdown
Collaborator Author

@musikele, for this patch I would prefer to revert to 1.13 behavior. For a proper fix I think it's necessary to do more testing, especially against stylesheets that have been built around 1.13.

musikele
musikele approved these changes Nov 9, 2022
View reviewed changes

@musikele musikele left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, the behaviour is the same of PUC 1.13

@musikele

musikele commented Nov 9, 2022

Copy link
Copy Markdown
Contributor

@dgirardi do you think I should open an issue to PUC for discussing this?

@dgirardi dgirardi mentioned this pull request Nov 9, 2022
Open
@dgirardi dgirardi mentioned this pull request Nov 22, 2022
Merged
@dgirardi dgirardi merged commit 84be815 into prebid:patch-1.14 Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@musikele musikele musikele approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dgirardi @musikele