search

washingtonpost / ArcAds

ArcAds is a DFP wrapper created by Arc XP with publishers in mind.
https://www.npmjs.com/package/arcads
MIT License
57 stars 42 forks source link

Resize event fires without resizing past a breakpoint #82

Closed darimay closed 3 years ago

darimay commented 3 years ago

Hi,

I'm with CMG and I've noticed the following bug with ArcAds that is impacting ads revenue for us.

Expected Behavior

The resize based fetchBids/refreshSlot requests should trigger only when the page is resized beyond a breakpoint.

Actual Behavior

The resize based fetchBids/refreshSlot requests are triggered on first resize event on the page regardless of window width (Sometimes the resize event is triggered by the browser on page load, without any actual change in the window's width, however, this is not an ArcAds bug, but it certainly exacerbates the ArcAds bug described).

Steps to Reproduce the Behavior in Chrome

  1. Ensure all ad blocking plugins are disabled.
  2. Open the Network tab and monitor ad requests by filtering for /gampad.+hp01/ for the HP01 slot or any other slot on the page.
  3. With the Network tab open, visit https://www.wsbtv.com/.
  4. Note that there are two requests made after page fully loads.
  5. If you trace the second request, it's originated in the sizemapping logic

Additional Comments

When a user resizes the window's width for the first time after page load, the callback produced by runResizeEvents should only fire if the width change is past a breakpoint, as described in the docs for the function:

Resize event that checks if a user has resized past a breakpoint included in the advertisements sizemap.

However, because the initial value of lastBreakpoint is undefined, then the lastBreakpoint !== breakpoint condition is satisfied as well as (width > breakpoint && (width < nextBreakpoint || !nextBreakpoint) for one breakpoint in [0, 768, 1024] on this line.

As a result fetchBids/refreshSlot is triggered unnecessarily leading to unnecessary ad requests for each ad slot.

Note that once lastBreakpoint is set after the initial call, this incorrect behavior goes away.

But to make matters worse, some desktop browsers trigger a resize event on the window after page load and because runResizeEvents is added to the window's resize EventListener, all ad slot requests are doubled, once on page load, and once on the subsequent unexpected resize event on the window that follows.

I have created the following Pull Request to address the issue: https://github.com/washingtonpost/ArcAds/pull/83.

Please advise on how we should proceed.

darimay commented 3 years ago

@nealmalkaniwapo

We've run into an issue caused by this merge. I've provided more details in the original PR: https://github.com/washingtonpost/ArcAds/pull/83

ghost commented 3 years ago

@darimay 4.0.1 has been released to address this issue