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

Add sanitizer to disable auto lightbox #6936

Merged
merged 9 commits into from
Mar 9, 2022
Merged

Add sanitizer to disable auto lightbox #6936

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f9f096b
Add sanitiser to disable auto lightbox
dhaval-parekh Feb 24, 2022
e9766ae
update unit test case
dhaval-parekh Feb 24, 2022
42de8a9
Remove unwanted code from unit test cases
dhaval-parekh Mar 8, 2022
13e68b2
Update condition to enable auto lightbox disabled sanitizer
dhaval-parekh Mar 8, 2022
71a113c
Add unit test cases
dhaval-parekh Mar 8, 2022
12f0c94
Disable lightbox for images by default
dhaval-parekh Mar 9, 2022
2624459
Merge branch 'develop' of github.com:ampproject/amp-wp into enhanceme…
westonruter Mar 9, 2022
e02a9f9
Ensure consistent application of AMP_Auto_Lightbox_Disable_Sanitizer
westonruter Mar 9, 2022
7a3d259
Test amp_auto_lightbox_disabled being filtered true
westonruter Mar 9, 2022
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
17 changes: 17 additions & 0 deletions includes/amp-helper-functions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1609,6 +1609,22 @@ function amp_get_content_sanitizers( $post = null ) {
);
}

westonruter marked this conversation as resolved.
Show resolved Hide resolved
/**
* Filters whether AMP auto-lightbox is disabled.
*
* When disabled, the data-amp-auto-lightbox-disable attribute is added to the body.
*
* @since 2.2.2
* @link https://github.com/ampproject/amphtml/blob/420bc3987f69f6d9cd36e31c013fc9eea4f1b245/docs/spec/auto-lightbox.md#disabling-treatment-explicitly
*
* @param bool $disabled Whether disabled.
*/
$is_auto_lightbox_disabled = apply_filters( 'amp_auto_lightbox_disabled', true );

if ( $is_auto_lightbox_disabled ) {
$sanitizers[ AMP_Auto_Lightbox_Disable_Sanitizer::class ] = [];
}

/**
* Filters the content sanitizers.
*
Expand Down Expand Up @@ -1688,6 +1704,7 @@ function amp_get_content_sanitizers( $post = null ) {

// Force core essential sanitizers to appear at the end at the end, with non-essential and third-party sanitizers appearing before.
$expected_final_sanitizer_order = [
AMP_Auto_Lightbox_Disable_Sanitizer::class,
AMP_Core_Theme_Sanitizer::class, // Must come before script sanitizer since onclick attributes are removed.
AMP_Bento_Sanitizer::class, // Bento scripts may be preserved here.
AMP_Script_Sanitizer::class, // Must come before sanitizers for images, videos, audios, comments, forms, and styles.
Expand Down
24 changes: 24 additions & 0 deletions includes/sanitizers/class-amp-auto-lightbox-disable-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php
/**
* Class AMP_Auto_Lightbox_Disable_Sanitizer
*
* @package AmpProject\AmpWP
*/

/**
* Disable auto lightbox for images.
*
* @since 2.2.2
* @internal
*/
class AMP_Auto_Lightbox_Disable_Sanitizer extends AMP_Base_Sanitizer {

/**
* Add "data-amp-auto-lightbox-disable" attribute to body tag.
*
* @return void
*/
public function sanitize() {
$this->dom->body->setAttributeNode( $this->dom->createAttribute( 'data-amp-auto-lightbox-disable' ) );
delawski marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Very good to use createAttribute as then no attribute value will be shown. 👍

}
}
15 changes: 15 additions & 0 deletions tests/php/test-amp-helper-functions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1907,6 +1907,21 @@ static function( $classes ) {
$this->assertEquals( AMP_Tag_And_Attribute_Sanitizer::class, $ordered_sanitizers[ count( $ordered_sanitizers ) - 1 ] );
}

/**
* @covers ::amp_get_content_sanitizers()
*/
public function test_amp_get_content_sanitizers_for_lightbox_sanitizer() {
$this->assertArrayHasKey( AMP_Auto_Lightbox_Disable_Sanitizer::class, amp_get_content_sanitizers() );

add_filter( 'amp_auto_lightbox_disabled', '__return_false', 10 );

$this->assertArrayNotHasKey( AMP_Auto_Lightbox_Disable_Sanitizer::class, amp_get_content_sanitizers() );

add_filter( 'amp_auto_lightbox_disabled', '__return_true', 20 );

$this->assertArrayHasKey( AMP_Auto_Lightbox_Disable_Sanitizer::class, amp_get_content_sanitizers() );
}

/**
* Test amp_get_content_sanitizers().
*
Expand Down
30 changes: 30 additions & 0 deletions tests/php/test-class-amp-auto-lightbox-disable-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php
/**
* Class AMP_Auto_Lightbox_Disable_Sanitizer_Test.
*
* @package AmpProject\AmpWP
*/

use AmpProject\AmpWP\Tests\TestCase;

/**
* Tests the auto lightbox disable sanitizer class.
*
* @coversDefaultClass AMP_Auto_Lightbox_Disable_Sanitizer
*/
class AMP_Auto_Lightbox_Disable_Sanitizer_Test extends TestCase {

/**
* @covers ::sanitize()
*/
public function test_sanitize() {
delawski marked this conversation as resolved.
Show resolved Hide resolved

$source = '<html><body class="body-class"><span>Hello World!</span></body></html>';
$dom = AMP_DOM_Utils::get_dom_from_content( $source );

$sanitizer = new AMP_Auto_Lightbox_Disable_Sanitizer( $dom );
$sanitizer->sanitize();

$this->assertTrue( $dom->body->hasAttribute( 'data-amp-auto-lightbox-disable' ) );
}
}