IE fix: calling matchMedia with an empty string

[benjaminclot]

Type of change

• Bugfix

Description of change

In IE, calling matchMedia with an empty string throws an error.

[benjaminclot]

As a side note, this should also be merged into the 2.44.x-legacy branch

[Fawke]

Hi @benjaminclot,

I see your point, when we have a sizeConfig object like this:

pbjs.setConfig([{
  sizeConfig: {
    mediaQuery: '',
    sizesSupported: [
      [728, 90],
      [300, 250]
    ],
    labels: ['desktop', 'tablet']
  }
}]);

Currently, we don't have any checks in place to indicate to the user that mediaQuery string is declared an empty string.

Ideally, this shouldn't be allowed, i.e, providing an empty string for mediaQuery property. I've checked,
window.matchMedia(''), it evaluates to true in Chrome browser. In IE, as you said, it throws an error, invalid argument!

With your approach, where you are setting ruleMatch to true if mediaQuery string is empty, what would happen is all the sizes in sizesSupported will be set to true and those sizes will be available for selection when the intersection of mediaTypes.banner.sizes and sizesSupported is calculated.

For example, consider this ad unit setup.

{
    code: "ad-slot-1",
    mediaTypes: {
        banner: {
            sizes: [
                [970, 90],
                [728, 90],
                [300, 250],
                [300, 100]
            ]
        }
    },
    bids: [{
            bidder: "pulsepoint",
            labelAny: ["desktop", "tablet"],
            params: {
                "cf": "728X90",
                "cp": 123456,
                "ct": 123456
            }
        },
        {
            bidder: "pulsepoint",
            labelAny: ["desktop", "phone"],
            params: {
                "cf": "300x250",
                "cp": 123456,
                "ct": 123456
            }
        }]
}

Let's say the user is accessing the webpage from a mobile phone and viewport width is 480px. In this case, since we have configured sizeConfig[0].mediaQuery with empty string, the mediaTypes.banner.sizes will equal [[728, 90], [300, 250]] after sizeConfig filtration process is complete. [728, 90] shouldn't be included, since our viewport width is only 480px. So, the request we sent to the adapter, and SSP would be wrong.

A simple fix, that I can think of now is:

if (
      typeof config === 'object' &&
      typeof config.mediaQuery === 'string' &&
      config.mediaQuery.length > 0
    ) {
      ...
      }
    }

We can just put this extra check in place config.mediaQuery.length > 0 in the if condition. This will block out all setups where mediaQuery field is declared as empty.

Let me know what you think.

[benjaminclot]

Hi @Fawke,

The goal of my proposition was to have IE behave like Chrome and not introduce a potential breaking change. In your example, you deduce that [728, 90] shouldn't be included but you don't effectively know that: maybe there are edge cases where people have been using this "feature", like including a [1, 1] size, whatever the viewport is 🤷‍♂

[Fawke]

@benjaminclot,

We'll review this internally with other prebid.js engineers and let them take a call if we should block this behaviour completely or allow it for IE (and other browsers).

[snapwich]

If this matches the behavior of chrome, then I think it's an appropriate fix for IE. LGTM

[mkendall07]

In this case, since the undefined behavior is behaving differently in different browsers, we should not make the breaking change unless it's in a major release. So for now this fix is good, but @Fawke we should make the breaking change in a future 4.x release. Thanks

[Fawke]

LGTM! Merging this master. This issue will be tracked and adding mediaQuery as an empty string would be disallowed from next major version of Prebid.js