Support for out of page (interstitial) ad slot

Can there be more than one of these declared on a page. It would be weird to create a block for this, but if we dont how do we get the correct slot defined on the page. ... I just want to think this workflow through a bit.

minor note:

+++ b/dfp.moduleundefined
@@ -716,7 +716,11 @@ function _dfp_js_global_settings() {
+  } else {

coding standard: else should be on a new line

Would it be incorrect to have more than one Out of page placement?

Thats actually my question ... would it?

The site I work on now has 2 out of page slots, one is an interstitial and the other is wallpaper. The wallpaper is also called rails or gutters. This is how the site was doing it and how the company 'Outsourced Ad Ops' recommend we do it.

Therefore to be flexible I believe this should allow for multiple out of page slots.

#13: 2038965-out_of_page_slot-6.patch queued for re-testing.

'#default_value' => $tag->settings['out_of_page'],

needs to be

'#default_value' => isset($tag->settings['out_of_page']) ? $tag->settings['out_of_page'] : 0,

Otherwise existing ads will throw an error when you try to edit them.

Also, when testing, I created an ad that had "out of page" checked and used admin/structure/block to add it to the page. The problem here is that all the normal block markup is added. With a vanilla install of D7 this means you can get this:

This needs to be handled more cleanly...

Updated patch with correct function name (googletag.defineOutOfPageSlot)

Updated patch with correct function name (googletag.defineOutOfPageSlot)

marcelovani ... I do not see the logic to deal with #17 in the patch you have in #20. Am I missing something?

...used CSS to make it not visible

Thats considered bad practice. We should really handle this in the hook_block_view() somehow. We might need to render the tag using a different template or something - I havent thought it through very much - but we should not be outputting any markup that later needs to be hidden with CSS.

Alternatively we can rethink how these ads are placed somehow and not use the block system at all, but use hook_page_alter (or something) but then you loose the benefits of using context to decide which page(s) these are displayed on. Hmmmm.

Thoughst?

We could add this to every page, maybe at the bottom, so don't need to worry about placing it using context.
I believe the Ads can be targeted to certain keyworks or pages, so it would be possible to choose which pages would show the ads.
Or, we could create a context plugin specifically for the Out of page slot.

My problem with this is that google charges sites based on the number of calls, even if no ad is returned. Adding this to every page would increase everyone's bill.

I think the correct solution would be to define a theme function that used a template with nothing in it except print $block->content and then when we define blocks in hook info we check for your new property and switch what theme function is used for these blocks. How bout that?

Not really sure about the new block template suggestion mechanism as it sounds like deviating from the normal block themeing and bit hacky? Also we need to ensure block caching and other components are unaffected.

@marcelovani and I have been discussing an idea to ditch the block system for out-of-page ad slots and instead place the required iframe via javascript. Dfp configuration could ask for paths to place the ad slot on which the JS should use, as a start. Later we could reuse the same to make a Context plugin. Would you see this working?

Deviating from the normal block theming is exactly what is called for here... That's the point. These ads shouldn't have a div around them. They should nave no physical presence on the page. A different block theme would not effect block caching and it would still allow for the use of the context module or panels to decide under what conditions these ad tags should be present on the page (e.g. Only show this out of page tag on Tuesdays)

Placing via js sounds like an interesting idea but again I'm concerned about context integration ... Definitely be willing to consider this route though

Ultimately this has the same flaw ... We are adding a bunch of stuff in one place just to remove it later.

Can you clarify your reasoning for not simply adding a new block theme function or tpl? I'm open to other ideas but I don't see why adding a new block theme function wouldn't work....

I took patch .20 and updated it for DFP 7.x-1.1 code

I guess i'm going to need some help; never submitted a patch before. I put the define slot in an array as I was having the issue where the machine name was a number or starting with a number and that was causing all sorts of issues. This is https://drupal.org/node/2078187 JS error when Ad Slot machine name starts with an integer.

we are using the production DFP module.

in the meantime, in the very very short term...

is there a simple explanation for what manual steps might be used to implement defineOutOfPageSlot , until such time as this patch is released into production?

can the DFP module still be used?

thanks.

Hi. When we check "Render ads asynchronously" in general dfp settings, outofpage tags become invisible. Also we get this js error - Uncaught ReferenceError: jQuery is not defined, while trying to access outofpage tags in async mode. How can I make jQuery loads before outofpage tags? I have tried to make jquery_update module weight equal to -10, but it gives nothing. Thank you!

Yes I have turned on and off agregation, also I had cleared caches. One thing that I miss is that I didn't put any div for outofpage inside body tag. I just put it as a block. If it is my mistake, could you please explain how can I do this, thanks!

Thank you, marcelovani. I will test this code and will tell you result. Thank you!

Thank you, marcelovani. This code works nice, and in synchronous ads rendering it seems to work much faster then block implementation. But my problem with async is still there, possibly it is an error on ads end, not in drupal. Thank you!

A couple of things:

* line 923:

We cannot assume that page_top will always exists.

* line 951:

$visible = !($tag->out_of_page_visibility xor $page_match);
should be:
$visible = !($tag->settings['out_of_page_visibility'] xor $page_match);

Patch in #42 is missing the dfp_context_reaction_outofpage.inc file.
Re-rolling.

Thank you @Marcelo, I think the main problem in my case is that jQuery is loaded after gpt.js. I will try to find problem and then report here, thx!

Thanks @marcelovani . Added these changes into the patch.
Also changed line 919:
$html .= render(dfp_tag($machinename));
to

$dfp_tag = dfp_tag($machinename);
$html .= render($dfp_tag);

So php 5.5 will not complain about passing variables by reference

This is working as expected

1. +++ b/dfp.module
   @@ -866,3 +883,73 @@ function template_preprocess_dfp_short_tag(&$variables) {
   +    drupal_add_css(implode(',', $selectors) . ' {position: absolute; height: 1px; padding: 0; margin: 0;}', array('group' => CSS_THEME, 'type' => 'inline'));
   

   This makes me cringe ... cant we just take advantage of the "element-hidden" class instead?

2. +++ b/plugins/contexts/dfp_context_reaction_outofpage.inc
   @@ -0,0 +1,57 @@
   +      '#description' => t('The following DFP tags will be added to the page.'),
   

   There needs to be some messaging added here if $options is empty

3. +++ b/plugins/export_ui/dfp_ctools_export_ui.inc
   @@ -89,13 +89,28 @@ function dfp_tag_form(&$form, &$form_state) {
   +  );
   ...
   +  $form['tag_settings']['out_of_page'] = array(
   +    '#type' => 'checkbox',
   +    '#title' => t('Out of page (interstitial) ad slot'),
   +    '#description' => (module_exists('context')) ?
   +        t('Use Context module to place the Ad slot on the page.') :
   +        t('The slot will be placed on pages, respecting the Page specific settings below.'),
   +    '#default_value' => isset($tag->settings['out_of_page']) ? $tag->settings['out_of_page'] : 0,
   +    '#parents' => array('settings', 'out_of_page'),
   +    '#weight' => 0,
   ...
   +    '#states' => array(
   +      'invisible' => array(
   +        'input[name="settings[out_of_page]"]' => array('checked' => TRUE),
   +      ),
   +    ),
   
   @@ -135,6 +150,11 @@ function dfp_tag_form(&$form, &$form_state) {
   +    '#states' => array(
   +      'invisible' => array(
   +        'input[name="settings[out_of_page]"]' => array('checked' => TRUE),
   +      ),
   +    ),
   
   +++ b/plugins/contexts/dfp_context_reaction_outofpage.inc
   @@ -0,0 +1,57 @@
   +    foreach ($tags as $tag) {
   +      if (!empty($tag->settings['out_of_page'])) {
   +        $options[$tag->machinename] = $tag->slot;
   +      }
   +    }
   

   Why bother having this at all? Why not just show a checkbox for all tags on the context reaction form?

4. +++ b/plugins/export_ui/dfp_ctools_export_ui.inc
   @@ -89,13 +89,28 @@ function dfp_tag_form(&$form, &$form_state) {
   +    '#required' => FALSE,
   
   @@ -236,6 +289,10 @@ function dfp_tag_form_validate(&$form, &$form_state) {
   +  // Size is required for non-interstitial slots.
   +  if (empty($form_state['values']['settings']['out_of_page']) && empty($form_state['values']['size'])) {
   +    form_set_error('size', t('Size field is required.'));
   +  }
   
   +++ b/plugins/export_ui/dfp_tag_ui.class.php
   @@ -54,6 +54,10 @@ class dfp_tag_ui extends ctools_export_ui {
   +    // Interstitial slots do not have Size attribute.
   +    if (!empty($item->settings['out_of_page'])) {
   +      $item->size = t('N/A');
   +    }
   
   @@ -91,6 +95,11 @@ class dfp_tag_ui extends ctools_export_ui {
   +      // Interstitial slots do not require size and should not be displayed as a block.
   +      if (!empty($form_state['values']['settings']['out_of_page'])) {
   +        $form_state['item']->size = '0';
   +        $form_state['item']->block = '0';
   +      }
   

   This is an API change I'm not comfortable with. I'd be ok with adding a note to the description that says "use 0x0 for out of page ads" or something like that

5. +++ b/plugins/export_ui/dfp_ctools_export_ui.inc
   @@ -143,6 +163,40 @@ function dfp_tag_form(&$form, &$form_state) {
   +  if (!module_exists('context')) {
   +    // Settings for visibility.
   +    $options = array(t('On every page except the listed pages.'), t('On the listed pages only.'));
   +    $description = t('Enter one page per line as Drupal paths. The "*" character is a wildcard. ' .
   +      'Example paths are %blog for the blog page and %blog-wildcard for every personal blog. ' .
   +      '%front is the front page.',
   +      array('%blog' => 'blog', '%blog-wildcard' => 'blog/*', '%front' => '<front>'));
   +    $form['tag_settings']['visibility'] = array(
   +      '#type' => 'fieldset',
   +      '#title' => t('Page specific settings'),
   +      '#collapsible' => TRUE,
   +      '#collapsed' => FALSE,
   +      '#states' => array(
   +        'invisible' => array(
   +          'input[name="settings[out_of_page]"]' => array('checked' => FALSE),
   +        ),
   +      ),
   +    );
   +    $form['tag_settings']['visibility']['dfp_outofpage_visibility'] = array(
   +      '#type' => 'radios',
   +      '#title' => t('Add this Ad slot'),
   +      '#options' => $options,
   +      '#parents' => array('settings', 'out_of_page_visibility'),
   +      '#default_value' => isset($tag->settings['out_of_page_visibility']) ? $tag->settings['out_of_page_visibility'] : 0,
   +    );
   +    $form['tag_settings']['visibility']['dfp_outofpage_pages'] = array(
   +      '#type' => 'textarea',
   +      '#title' => t('Pages'),
   +      '#parents' => array('settings', 'out_of_page_pages'),
   +      '#default_value' => isset($tag->settings['out_of_page_pages']) ? $tag->settings['out_of_page_pages'] : 'admin*' . PHP_EOL,
   +      '#description' => $description,
   +      '#wysiwyg' => FALSE,
   +    );
   +  }
   

   First, what is this? What does this have to do with "out of page" ads? If I understand this correctly, everything that is here can be accomplished with the "Context" module. Why do we need code to handle visibility?

6. +++ b/dfp.module
   @@ -866,3 +883,73 @@ function template_preprocess_dfp_short_tag(&$variables) {
   \ No newline at end of file
   

   nit: new line at EOF

7. +++ b/dfp.module
   @@ -866,3 +883,73 @@ function template_preprocess_dfp_short_tag(&$variables) {
   +function dfp_page_alter(&$page) {
   

   I suspect that based on my other comments at least 90% of this function can be removed

8. +++ b/dfp.module
   @@ -866,3 +883,73 @@ function template_preprocess_dfp_short_tag(&$variables) {
   +function _dfp_visibility_pages($tag) {
   

   Based on my other comments, this function should be unneeded

1 and 2: We could try it.

Cool

3: I just thought would be nice to display only options that were relevant for the type of Ad slot selected.

We are just adding complexity in the name of simplicity. I think the simplicity is minimal but the complexity is a lot of code to baby-sit

4: Agree with 0x0.

Cool

5: We can remove it if you think it's not important, the ide was to provide functionality for people that don't user Context.

I dont want to babysit that code either if there is a good solution already

+++ b/dfp.module
@@ -866,3 +883,28 @@ function template_preprocess_dfp_short_tag(&$variables) {
+  $page['page_top']['dfp_out_of_page'] = array(

As far as I know, nothing guarantees that "page top" exists. It's annoying, but I think that "content" is the only guaranteed region. That said I believe you can just add "page_top" if it doesnt exist in $page already

Other than this, I'm good to go :)

Thank you marcelovani for your patch. But I have a question. What if I need to combine couple out of page slots from different contexts. In current out of page reaction only first context value added. To make possible addition of multiple out of page slots from different contexts we need to remove break from context execute() method.

Thanks for the quick response, anything from stopping this going into dev.

Cheers Dan

@danharper, have you tested the patch in #66? If so, can you report back what you did to test and what the results were...

1. +++ b/dfp.module
   @@ -465,12 +477,16 @@ function dfp_tag_load($machinename) {
   -  ctools_include('export');
   +  $tags = &drupal_static(__FUNCTION__, array());
    
   -  $tags = ctools_export_crud_load_all('dfp_tags');
   -  foreach ($tags as $key => $tag) {
   -    if (!$include_disabled && isset($tag->disabled) && $tag->disabled) {
   -      unset($tags[$key]);
   +  if (empty($tags)) {
   +    ctools_include('export');
   +
   +    $tags = ctools_export_crud_load_all('dfp_tags');
   +    foreach ($tags as $key => $tag) {
   +      if (!$include_disabled && isset($tag->disabled) && $tag->disabled) {
   +        unset($tags[$key]);
   +      }
   

   The problem here is that ctools_export_crud_load_all() already stores these results in a static variable. Adding this will cause all the entities to be stored in memory twice.

2. +++ b/dfp.module
   @@ -886,3 +906,38 @@ function template_preprocess_dfp_short_tag(&$variables) {
   +  foreach ($tags as $tag) {
   +    if (isset($tag->settings['out_of_page']) && $tag->settings['out_of_page'] == TRUE) {
   +      drupal_alter('dfp_tag_load', $tag);
   +    }
   +  }
   +
   

   why? This alter is already called on load.

Re 1: If you think it's not needed, no problem, we can revert it.

Yeah ... there is no need to add to the memory footprint. Lets kill it

Re 2: If you remove it and try to run the code, you will have no Out of page tags on the page. I need to call drupal_alter to process all the Out of page tags since they do not get processed by the block hooks anymore. How about we put the drupal_alter('dfp_tag_load', $tag) into dfp_tag_load_all()?

I dont think we can move it to dfp_tag_load_all because there are plenty of reasons why people would call dfp_tag_load and the alter should really run. That said, you are running dfp_tag_load() just a few lines down from this code anyway.

+++ b/dfp.module
@@ -886,3 +906,38 @@ function template_preprocess_dfp_short_tag(&$variables) {
+  }

On second glance I dont see why this is here at all. You dont use the $tags variable anywhere

I think #75 looks good to go ... I'd love to have at least one more person give the thumbs up, but if no one steps up soon I will commit this.

#75 works fine for me, thanks for the patch Marcelo!

The patch in #75 doesn't apply to the latest dev but I was able to apply it manually. So far everything seems to look good. I created the ad slot and used the context module to place it on the homepage and a specific node. I see the code in the correct places. I've notified my DFP manager to start testing serving to it but I'm not getting any errors so I think this is a good patch.

I will wait and confirm that ads are serving properly and I can do a reroll at that point. Would be great if this can get commited soon so I don't have to maintain another patched module.

Ok ... I took this for another test drive today. All is well. Thanks everyone!!!

Hey Guys, I know this thread has been closed for some time now.

I'm working with a client who is trying to serve interstitial adverts on their site. I have gone through the configuration on the dfp module side and according to everything I've read, and based on the usage example at the top of the page here, it is setup correctly on our end.

My question, does anyone have experience setting up the ad slot on the google dfp side? This is proving challenging since I do not have access to their ad campaign/slot configurations. But it seems to me something is not setup correctly on the ad slot itself vs drupal.

Also note, I'm using a hook_page_build to render this ad slot, since context module is heavy and I'd rather not install it.

/**
 * Implements of hook_page_build().
 * DFP Interstitial ad placement without context module
 */
function my_module_page_build(&$page) {
  $html = '';
  $machinename = '';
  $tags = dfp_tag_load_all();
  foreach ($tags as $tag) {
    if (isset($tag->settings['out_of_page']) && $tag->settings['out_of_page'] == TRUE && $tag->machinename == 'interstitial') {
          drupal_alter('dfp_tag_load', $tag);
      $machinename = $tag->machinename;
    }
  }
  if (!empty($machinename)) {
    $tag = dfp_tag_load($machinename);
    if (empty($tag->disabled)) {
      $dfp_tag = dfp_tag($machinename);
      $dfp_tag['dfp_wrapper']['#attributes']['class'][] = 'element-hidden';
      $html .= render($dfp_tag);
    }
  }
  // Add Out Of Page slots on the top of the page.
  $page['page_top']['dfp_out_of_page'] = array(
      '#weight' => -1000,
      '#markup' => $html,
  );
}

output in head

<script type="text/javascript">
<!--//--><![CDATA[//><!--
googletag.slots["interstitial"] = googletag.defineOutOfPageSlot("/NETWORK-ID/CAMPAIGN/interstitial", "dfp-ad-interstitial")
  .addService(googletag.pubads());
//--><!]]>
</script>

output in body
I see the advert markup is rendering properly, however, it is not behaving as a popup. The element-hidden is still applied to the parent wrapper, and the advert slot (iframe) is coming through as 1px x 1px... Seems something is not triggering this to actually behave like a popup on the advert side. I've been told an out of page 'pop-up, pop-under' template has been configured on the advert side of things in Google.

Any insights would be very appreciated.

Thanks