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

device.(w|h): use screen.(width|height) instead of (innerWidth|innerHeight) #12065

Closed
justadreamer opened this issue Jul 31, 2024 · 2 comments · Fixed by #12108
Closed

device.(w|h): use screen.(width|height) instead of (innerWidth|innerHeight) #12065

justadreamer opened this issue Jul 31, 2024 · 2 comments · Fixed by #12108

Comments

@justadreamer
Copy link
Contributor

Type of issue

enhancement

Description

The proposed change is to replace these lines in src/fpd/enrichment.js:

      const w = win.innerWidth || win.document.documentElement.clientWidth || win.document.body.clientWidth;
      const h = win.innerHeight || win.document.documentElement.clientHeight || win.document.body.clientHeight;

with

      const w = window.screen.width;
      const h = window.screen.height;

Motivation

  1. Consistency, DRYness. Many adapters already do this anyway overriding the fpd implementation (f.e. see ixBidAdapter, adplusBidAdapter, adxcgAnalyticsAdapter, adxpremiumAnalyticsAdapter, brightMountainMediaBidAdapter, sharethroughBidAdapter, saambaaBidAdapter)
  2. Following the standard. oRTB spec actually prescribes that device.(w|h) is the physical size of the screen in pixels, not browser window size
    image

Alternative

Remove this code from the src/fpd/enrichment.js completely and let FPD module / RTD submodules take care of this.

Other considerations

(can be extracted into a separate issue)

Given oRTB spec literally says "physical size of the screen in pixels" - perhaps it makes more sense to set it in physical pixels, not logical aka CSS pixels? Then it would be:

      const pxratio = window.devicePixelRatio || 1;
      const w = window.screen.width * pxratio;
      const h = window.screen.height * pxratio;

devicePixelRatio is usually more than 1 on retina and HD displays. Thus it is desirable to either satisfy the spec or make it more precise if the industry expects device.(w|h) value to be expressed in CSS pixels.

cc: thanks @patmmccann and @BohdanVV for raising this point as part of this PR review comment #12005 (comment)

@patmmccann
Copy link
Collaborator

Interesting information from Safari as an aside
image

@patmmccann
Copy link
Collaborator

proposal extension: continue to pass inner height and width as device.ext.vpw and vph

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging a pull request may close this issue.

2 participants