search

InteractiveAdvertisingBureau / openvv

Other
81 stars 50 forks source link

fix bug detecting all windows OS versions as >= 8.1, fix OVV version incompatibility #97

Closed advir1 closed 9 years ago

advir1 commented 9 years ago

The function was trying to detect the OS using the version property, instead of the os property

advir1 commented 9 years ago

When the page originally adds an outdated (version 1.0 / 1.1) OVV object, it remains as a singleton with properties missing for OVV 1.2 script (IN_XD_IFRAME for example). OVV 1.2 consequentially fails to run, and the asset is never added, which causes an error when looking up the asset to measure viewability.

jdreetz commented 9 years ago

@alonashkenazi do we have an eta for when this can merged in?

goosemanjack commented 9 years ago

I am just becoming familiar with this codebase now. Is the feeling that we should also start incrementing that PATCH number in the version number as we accept these pull requests ( http://semver.org/ )

Additionally, I don't currently have an environment set up to verify these patches, so I'll need to coordinate with you for verification once the patch is applied.

goosemanjack commented 9 years ago

I merged this fix, but did not increment the version number yet. Merged code must be verified by advir1 until we get testing capacity in place.