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

consentmanager migration #5152

Merged
merged 13 commits into from
May 17, 2024
Merged

consentmanager migration #5152

merged 13 commits into from
May 17, 2024

Conversation

muuki88
Copy link
Contributor

@muuki88 muuki88 commented Feb 11, 2024

Migration from OneTrust to consentmanager.net for cookie consent.

#5149

Copy link

netlify bot commented Feb 11, 2024

Deploy Preview for prebid-docs-preview ready!

Name Link
🔨 Latest commit cceaaa8
🔍 Latest deploy log https://app.netlify.com/sites/prebid-docs-preview/deploys/66466e492c86c20008bf7806
😎 Deploy Preview https://deploy-preview-5152--prebid-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@muuki88 muuki88 marked this pull request as draft February 11, 2024 10:23
@muuki88
Copy link
Contributor Author

muuki88 commented Mar 12, 2024

Cookies previously defined in OneTrust

image

image

@muuki88 muuki88 marked this pull request as ready for review March 12, 2024 20:32
Copy link
Contributor Author

@muuki88 muuki88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bretg everything works now.

The cookie policy page will be updated once I have added the missing vendors. I would create an account at consentmanager for you as well.

Copy link
Contributor

@bretg bretg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice progress. might need to spend some time with you to understand this new CMP interaction @muuki88

_includes/prebidjs-non-prod.html Outdated Show resolved Hide resolved
<script async src="//cdn.jsdelivr.net/npm/prebid.js@latest/dist/not-for-prod/prebid.js"></script>
<script type="text/javascript" src="//services.brid.tv/player/build/brid.min.js"></script>

<script type="text/plain" class="cmplazyload" data-cmp-purpose="c51" src="//services.brid.tv/player/build/brid.min.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is there a call to the Brid player in this generic-looking include file?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@muuki88 - why is this generic page loading Brid?

_includes/video/pb-cp.html Outdated Show resolved Hide resolved
_includes/video/pb-is-amp.html Outdated Show resolved Hide resolved
_includes/video/pb-is-bc.html Outdated Show resolved Hide resolved
_includes/video/pb-os-nas.html Show resolved Hide resolved
_includes/video/pb-vm-vjs.html Show resolved Hide resolved
_includes/video/pbs-jw01.html Outdated Show resolved Hide resolved
_includes/video/pbs-kl.html Outdated Show resolved Hide resolved
_includes/video/pbs-rd.html Outdated Show resolved Hide resolved
Co-authored-by: bretg <bgorsline@gmail.com>
@muuki88
Copy link
Contributor Author

muuki88 commented Apr 9, 2024

https://docs.prebid.org/prebid-video/video-integrating-solo.html keep the linked ones here and remove the rest.

@bretg
Copy link
Contributor

bretg commented May 2, 2024

@muuki88 - looked things over. The remaining questions are about the video examples.

Neither of these work for me locally

  • pb-ve-jwplayer-platform.html
  • pb-ve-jwplayer-hosted.html

This shows the player regardless of cookie settings

  • pb-ve-videojs.html

This page is titled oddly and doesn’t make sense where it is in the nav bar

  • /prebid-video/video-integrating-solo.html

The Vimeo videos aren’t affected. Not following _includes/vimeo-iframe.html

@muuki88
Copy link
Contributor Author

muuki88 commented May 2, 2024

Thanks @bretg for taking a look. most of the examples did not work for me neither, but as far as I could tell, consent was not the issue.

Regarding the naming, I'll take a look when I'm back from London.

@bretg
Copy link
Contributor

bretg commented May 2, 2024

Some of these problems (e.g. the odd title) exist in the current site, so we can fix that in a separate PR.

The video examples like https://docs.prebid.org/examples/video/instream/jwplayer/pb-ve-jwplayer-platform.html don't work currently either. Maybe we should just merge this PR and work with the video team to repair the examples -- maybe refactoring them to not require actual bids at the same time?

@muuki88
Copy link
Contributor Author

muuki88 commented May 10, 2024

What I can see is not working is the JW player that is missing a licence key. From a technical perspective the pipes work 😬

https://deploy-preview-5152--prebid-docs-preview.netlify.app/examples/video/instream/jwplayer/pb-ve-jwplayer-hosted

image

Maybe we should just merge this PR and work with the video team to repair the examples -- maybe refactoring them to not require actual bids at the same time?

Yes. I'm all for that. I would phrase it that the examples are less broken, because the consent stuff at least works now.

@muuki88
Copy link
Contributor Author

muuki88 commented May 10, 2024

This shows the player regardless of cookie settings

  • pb-ve-videojs.html

And it should, because it's a native HTML element 🎉

      <video
        id="vid1"
        class="video-js vjs-default-skin vjs-big-play-centered"
        controls
        data-setup="{}"
        width="640"
        height="480"
      >
        <source src="https://vjs.zencdn.net/v/oceans.mp4" type="video/mp4" />
        <source src="https://vjs.zencdn.net/v/oceans.webm" type="video/webm" />
        <source src="https://vjs.zencdn.net/v/oceans.ogv" type="video/ogg" />
      </video>

There's no additional player loaded and no ads if you don't load it. I could however hide it behind consent to make it clearer.

@muuki88
Copy link
Contributor Author

muuki88 commented May 10, 2024

This page is titled oddly and doesn’t make sense where it is in the nav bar

  • /prebid-video/video-integrating-solo.html

image

The naming is indeed a bit odd. The place in the sidebar however is correct. I'll open another issue to tackle this. Changing the URL structure is IMHO out of scope here.

The Vimeo videos aren’t affected. Not following _includes/vimeo-iframe.html

I don't quite get this 😅 Is this a question, request or just a statement?

@muuki88 muuki88 requested a review from bretg May 16, 2024 13:54
@muuki88 muuki88 changed the title Work in progress consentmanager migration consentmanager migration May 16, 2024
@muuki88
Copy link
Contributor Author

muuki88 commented May 16, 2024

@bretg . It is done 🎉 . All vimeo videos are now also behind the consent check

Copy link
Contributor

@bretg bretg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!

<script async src="//cdn.jsdelivr.net/npm/prebid.js@latest/dist/not-for-prod/prebid.js"></script>
<script type="text/javascript" src="//services.brid.tv/player/build/brid.min.js"></script>

<script type="text/plain" class="cmplazyload" data-cmp-purpose="c51" src="//services.brid.tv/player/build/brid.min.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@muuki88 - why is this generic page loading Brid?

@bretg
Copy link
Contributor

bretg commented May 17, 2024

merging

@bretg bretg merged commit 62131cb into master May 17, 2024
5 checks passed
@bretg bretg deleted the consentmanager branch May 17, 2024 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants