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

dgirardi commented 1 year ago

Addresses https://github.com/prebid/Prebid.js/issues/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>

musikele commented 1 year ago

@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:

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.

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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

prebid / prebid-universal-creative

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