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.

Sign up for GitHub

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

WordAds Block: AMP Support #12733

Merged
merged 14 commits into from
Aug 26, 2019
Merged

WordAds Block: AMP Support #12733

Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
5217499
Basic AMP support for Ads block.
jeffersonrabb Jun 18, 2019
46d9610
Sequential section IDs for amp-ad units, starting with 1.
jeffersonrabb Jun 18, 2019
faf35a8
Slight code reduction.
jeffersonrabb Jun 18, 2019
6538ce0
Extracted AMP check into separate function
dbspringer Jun 18, 2019
15e9b73
Added padding for "Report this ad"
dbspringer Jun 18, 2019
f9c876e
Fixed @since version
dbspringer Jun 18, 2019
7ac055e
Escaped attrs
dbspringer Jun 18, 2019
4daa277
Handle header ad insertion in AMP environment.
jeffersonrabb Jun 19, 2019
996dee2
Call is_amp() statically.
jeffersonrabb Jun 19, 2019
21a71ef
Header ad: skip House ads in AMP.
jeffersonrabb Jun 19, 2019
29d3d97
For "Top of each page" placements in AMP, use 320x50 unit which can s…
jeffersonrabb Jun 19, 2019
dedf03f
Suppress two additional script includes in AMP environment.
jeffersonrabb Jun 20, 2019
65ae218
Change initialization action to 'wp' to address _doing_it_wrong error…
jeffersonrabb Jul 26, 2019
2349a05
Initialize WordAds at rest_api_init action, for error-free behavior i…
jeffersonrabb Aug 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 88 additions & 13 deletions modules/wordads/wordads.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,32 @@ class WordAds {
'inline-plugin' => 320,
);

/**
* Counter to enable unique, sequential section IDs for all amp-ad units
*
* @var int
*/
public static $amp_section_id = 1;

/**
* Checks for AMP support and returns true iff active & AMP request
* @return boolean True if supported AMP request
*
* @since 7.5.0
*/
public static function is_amp() {
return class_exists( 'Jetpack_AMP_Support' ) && Jetpack_AMP_Support::is_amp_request();
}

/**
* Increment the AMP section ID and return the value
*
* @return int
*/
public static function get_amp_section_id() {
return self::$amp_section_id++;
}

public static $SOLO_UNIT_CSS = 'float:left;margin-right:5px;margin-top:0px;';

/**
Expand Down Expand Up @@ -106,7 +132,8 @@ function get_solo_unit_css() {
* @since 4.5.0
*/
function __construct() {
add_action( 'init', array( $this, 'init' ) );
add_action( 'wp', array( $this, 'init' ) );
add_action( 'rest_api_init', array( $this, 'init' ) );
}

/**
Expand Down Expand Up @@ -206,15 +233,19 @@ private function insert_adcode() {
}

if ( $this->option( 'enable_header_ad', true ) ) {
switch ( get_stylesheet() ) {
case 'twentyseventeen':
case 'twentyfifteen':
case 'twentyfourteen':
add_action( 'wp_footer', array( $this, 'insert_header_ad_special' ) );
break;
default:
add_action( 'wp_head', array( $this, 'insert_header_ad' ), 100 );
break;
if ( self::is_amp() ) {
jeffersonrabb marked this conversation as resolved.
Show resolved Hide resolved
add_filter( 'the_content', array( $this, 'insert_header_ad_amp' ) );
} else {
switch ( get_stylesheet() ) {
case 'twentyseventeen':
case 'twentyfifteen':
case 'twentyfourteen':
add_action( 'wp_footer', array( $this, 'insert_header_ad_special' ) );
break;
default:
add_action( 'wp_head', array( $this, 'insert_header_ad' ), 100 );
break;
}
}
}
}
Expand All @@ -239,6 +270,9 @@ function enqueue_scripts() {
* @return [type] [description]
*/
function insert_head_meta() {
if ( self::is_amp() ) {
return;
}
$themename = esc_js( get_stylesheet() );
$pagetype = intval( $this->params->get_page_type_ipw() );
$data_tags = ( $this->params->cloudflare ) ? ' data-cfasync="false"' : '';
Expand All @@ -261,6 +295,9 @@ function insert_head_meta() {
* @since 4.5.0
*/
function insert_head_iponweb() {
if ( self::is_amp() ) {
return;
}
$data_tags = ( $this->params->cloudflare ) ? ' data-cfasync="false"' : '';
echo <<<HTML
<link rel='dns-prefetch' href='//s.pubmine.com' />
Expand Down Expand Up @@ -396,11 +433,30 @@ function insert_header_ad_special() {

$ad_type = $this->option( 'wordads_house' ) ? 'house' : 'iponweb';
echo $this->get_ad( 'top', $ad_type );
echo <<<HTML
if ( ! self::is_amp() ) {
echo <<<HTML
<script type="text/javascript">
jQuery('.wpcnt-header').insertBefore('$selector');
</script>
HTML;
}
}

/**
* Header unit for AMP
*
* @param string $content Content of the page.
*
* @since 7.5.0
*/
public function insert_header_ad_amp( $content ) {

$ad_type = $this->option( 'wordads_house' ) ? 'house' : 'iponweb';
if ( 'house' === $ad_type ) {
return $content;
}
return $this->get_ad( 'top_amp', $ad_type ) . $content;

}

/**
Expand Down Expand Up @@ -456,6 +512,11 @@ function get_ad( $spot, $type = 'iponweb' ) {
} elseif ( 'inline' === $spot ) {
$section_id = 0 === $this->params->blog_id ? WORDADS_API_TEST_ID : $this->params->blog_id . '5';
$snippet = $this->get_ad_snippet( $section_id, $height, $width, $spot, self::$SOLO_UNIT_CSS );
} elseif ( 'top_amp' === $spot ) {
// 320x50 unit which can safely be inserted below title, above content in a variety of themes.
$width = 320;
$height = 50;
$snippet = $this->get_ad_snippet( null, $height, $width );
}
} elseif ( 'house' == $type ) {
$leaderboard = 'top' == $spot && ! $this->params->mobile_device;
Expand All @@ -481,14 +542,28 @@ function get_ad( $spot, $type = 'iponweb' ) {
*
* @since 5.7
*/
function get_ad_snippet( $section_id, $height, $width, $location = '', $css = '' ) {
public function get_ad_snippet( $section_id, $height, $width, $location = '', $css = '' ) {
$this->ads[] = array(
'location' => $location,
'width' => $width,
'height' => $height,
);
$ad_number = count( $this->ads ) . '-' . uniqid();

if ( self::is_amp() ) {
$height = esc_attr( $height + 15 ); // this will ensure enough padding for "Report this ad"
$width = esc_attr( $width );
$amp_section_id = esc_attr( self::get_amp_section_id() );
$site_id = esc_attr( $this->params->blog_id );
return <<<HTML
<amp-ad width="$width" height="$height"
type="pubmine"
data-siteid="$site_id"
data-section="$amp_section_id">
Copy link
Contributor

Choose a reason for hiding this comment

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

These should use esc_attr() no?

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, added.

</amp-ad>
HTML;
}

$ad_number = count( $this->ads ) . '-' . uniqid();
Copy link
Contributor

Choose a reason for hiding this comment

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

Using uniqid breaks the AMP plugin's post-processor cache. Better to use https://developer.wordpress.org/reference/functions/wp_unique_id/ or a polyfill.

Copy link
Contributor

Choose a reason for hiding this comment

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

Though I suppose this won't run in AMP anyway since it's after the return?

Copy link
Member

@dbspringer dbspringer Jun 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Though I suppose this won't run in AMP anyway since it's after the return?

That's correct.

$data_tags = $this->params->cloudflare ? ' data-cfasync="false"' : '';
$css = esc_attr( $css );

Expand Down