Skip to content

Commit

Permalink
Merge pull request #7560 from ampproject/fix/legacy-theme-img-respons…
Browse files Browse the repository at this point in the history
…iveness

Update legacy theme styles to add responsiveness to featured image
  • Loading branch information
westonruter authored Jul 7, 2023
2 parents 3007b13 + 892fa93 commit 3396a0c
Show file tree
Hide file tree
Showing 11 changed files with 123 additions and 55 deletions.
57 changes: 40 additions & 17 deletions .github/workflows/build-test-measure.yml
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,36 @@ jobs:
repo-token: '${{ secrets.GITHUB_TOKEN }}'
report-json: 'lint-js-report.json'

#-----------------------------------------------------------------------------------------------------------------------

normalize-composer:
name: 'Normalize composer.json'
needs: pre-run
if: needs.pre-run.outputs.changed-php-count > 0 || needs.pre-run.outputs.changed-gha-workflow-count > 0
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v3

- name: Setup PHP
uses: shivammathur/setup-php@v2
with:
php-version: '8.1'
coverage: none

- name: Get composer-normalize.phar
run: |
wget https://github.com/ergebnis/composer-normalize/releases/latest/download/composer-normalize.phar
chmod +x composer-normalize.phar
- name: Validate composer.json
run: composer --no-interaction validate --no-check-all

- name: Normalize composer.json
run: |
composer config --no-interaction --no-plugins allow-plugins.ergebnis/composer-normalize true
./composer-normalize.phar --dry-run
#-----------------------------------------------------------------------------------------------------------------------

lint-php:
Expand Down Expand Up @@ -189,18 +219,9 @@ jobs:
- name: Install Composer dependencies
run: composer install --prefer-dist --optimize-autoloader --no-progress --no-interaction

- name: Validate composer.json
run: composer --no-interaction validate --no-check-all

- name: Detect coding standard violations (PHPCS)
run: vendor/bin/phpcs -q --report=checkstyle --runtime-set ignore_errors_on_exit 1 --runtime-set ignore_warnings_on_exit 1 | cs2pr --graceful-warnings

- name: Normalize composer.json
run: |
composer config --no-interaction --no-plugins allow-plugins.ergebnis/composer-normalize true
composer require --no-interaction --dev ergebnis/composer-normalize --ignore-platform-reqs
composer --no-interaction normalize --dry-run
#-----------------------------------------------------------------------------------------------------------------------

static-analysis-php:
Expand Down Expand Up @@ -362,12 +383,13 @@ jobs:
mysql:
image: mariadb:latest
env:
MYSQL_ALLOW_EMPTY_PASSWORD: true
MYSQL_ROOT_PASSWORD:
MYSQL_DATABASE: wordpress_test
MARIADB_ALLOW_EMPTY_ROOT_PASSWORD: true
MARIADB_DATABASE: wordpress_test
MARIADB_MYSQL_LOCALHOST_USER: 1
MARIADB_MYSQL_LOCALHOST_GRANTS: USAGE
ports:
- 3306
options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3
options: --health-cmd="healthcheck.sh --su-mysql --connect --innodb_initialized" --health-interval=10s --health-timeout=5s --health-retries=3
continue-on-error: ${{ matrix.experimental == true }}
strategy:
fail-fast: false
Expand Down Expand Up @@ -606,12 +628,13 @@ jobs:
mysql:
image: mariadb:latest
env:
MYSQL_ALLOW_EMPTY_PASSWORD: true
MYSQL_ROOT_PASSWORD:
MYSQL_DATABASE: wordpress_test
MARIADB_ALLOW_EMPTY_ROOT_PASSWORD: true
MARIADB_DATABASE: wordpress_test
MARIADB_MYSQL_LOCALHOST_USER: 1
MARIADB_MYSQL_LOCALHOST_GRANTS: USAGE
ports:
- 3306
options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3
options: --health-cmd="healthcheck.sh --su-mysql --connect --innodb_initialized" --health-interval=10s --health-timeout=5s --health-retries=3
continue-on-error: ${{ matrix.experimental == true }}
strategy:
fail-fast: false
Expand Down
3 changes: 2 additions & 1 deletion bin/ci/install-wp-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ install_db() {
fi

# create database
mysqladmin create "$DB_NAME" --user="$DB_USER" --password="$DB_PASS""$EXTRA"
mariadb-admin create $DB_NAME --user="$DB_USER" --password="$DB_PASS"$EXTRA || \
mysqladmin create "$DB_NAME" --user="$DB_USER" --password="$DB_PASS"$EXTRA
}

install_wp
Expand Down
2 changes: 2 additions & 0 deletions includes/embeds/class-amp-core-block-handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,7 @@ private function process_text_widgets( Document $dom ) {
foreach ( $dom->xpath->query( '//div[ @class = "textwidget" ]' ) as $text_widget ) {
// Restore the width/height attributes which were preserved in preserve_widget_text_element_dimensions.
foreach ( $dom->xpath->query( sprintf( './/*[ @%s or @%s ]', self::AMP_PRESERVED_WIDTH_ATTRIBUTE_NAME, self::AMP_PRESERVED_HEIGHT_ATTRIBUTE_NAME ), $text_widget ) as $element ) {
/** @var DOMElement $element */
if ( $element->hasAttribute( self::AMP_PRESERVED_WIDTH_ATTRIBUTE_NAME ) ) {
$element->setAttribute( Attribute::WIDTH, $element->getAttribute( self::AMP_PRESERVED_WIDTH_ATTRIBUTE_NAME ) );
$element->removeAttribute( self::AMP_PRESERVED_WIDTH_ATTRIBUTE_NAME );
Expand All @@ -586,6 +587,7 @@ private function process_text_widgets( Document $dom ) {
* responsive so this is built-in. Note also the style rule for .wp-video in amp-default.css.
*/
foreach ( $dom->xpath->query( './/div[ @class = "wp-video" and @style ]', $text_widget ) as $element ) {
/** @var DOMElement $element */
$element->removeAttribute( 'style' );
}
}
Expand Down
2 changes: 2 additions & 0 deletions includes/embeds/class-amp-tumblr-embed-handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ public function sanitize_raw_embeds( Document $dom ) {
);
$overflow_element->textContent = __( 'See more', 'amp' );
$amp_element->appendChild( $overflow_element );

/** @var DOMElement $placeholder */
$placeholder->setAttribute( Attribute::PLACEHOLDER, '' );
$amp_element->appendChild( $placeholder );

Expand Down
1 change: 1 addition & 0 deletions includes/sanitizers/class-amp-style-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -1017,6 +1017,7 @@ public function sanitize() {
}
} else {
foreach ( $styled_elements as $element ) {
/** @var DOMElement $element */
$attr = $element->getAttributeNode( Attribute::STYLE );
if ( $attr && preg_match( '/!\s*important/i', $attr->value ) ) {
ValidationExemption::mark_node_as_px_verified( $attr );
Expand Down
57 changes: 33 additions & 24 deletions src/ReaderThemeSupportFeatures.php
Original file line number Diff line number Diff line change
Expand Up @@ -579,40 +579,49 @@ public function get_relative_luminance_from_hex( $hex ) {
* @return bool False if `wp_get_global_settings()` not exists or theme.json not found, true otherwise.
*/
private function theme_has_theme_json() {
if ( function_exists( 'wp_theme_has_theme_json' ) ) {
return wp_theme_has_theme_json();
// @TODO: Uncomment this once `wp_theme_has_theme_json()` caching is fixed.
// if ( function_exists( 'wp_theme_has_theme_json' ) ) {
// return wp_theme_has_theme_json();
// }

static $theme_has_support = null;
static $prev_stylesheet_directory = null;
static $prev_template_directory = null;

$stylesheet_directory = get_stylesheet_directory();
$template_directory = get_template_directory();

// Make sure that the cached $theme_has_support value is reset when the theme changes.
if ( null !== $theme_has_support && (
$stylesheet_directory !== $prev_stylesheet_directory ||
$template_directory !== $prev_template_directory
) ) {
$theme_has_support = null;
}

static $theme_has_support = null;
$prev_stylesheet_directory = $stylesheet_directory;
$prev_template_directory = $template_directory;

if (
null !== $theme_has_support &&

/*
* Ignore static cache when `WP_DEBUG` is enabled. Why? To avoid interfering with
* the theme developer's workflow.
*
* @todo Replace `WP_DEBUG` once an "in development mode" check is available in Core.
*/
! ( defined( 'WP_DEBUG' ) && WP_DEBUG ) &&

/*
* Ignore cache when automated test suites are running. Why? To ensure
* the static cache is reset between each test.
*/
! ( defined( 'WP_RUN_CORE_TESTS' ) && WP_RUN_CORE_TESTS )
// Ignore static cache when the development mode is set to 'theme', to avoid interfering with
// the theme developer's workflow.
( function_exists( 'wp_get_development_mode' ) && wp_get_development_mode() !== 'theme' )
) {
return $theme_has_support;
}

// Does the theme have its own theme.json?
$theme_has_support = is_readable( get_stylesheet_directory() . '/theme.json' );

// Look up the parent if the child does not have a theme.json.
if ( ! $theme_has_support ) {
$theme_has_support = is_readable( get_template_directory() . '/theme.json' );
// This is the same as get_theme_file_path(), which isn't available in load-styles.php context.
if ( $stylesheet_directory !== $template_directory && file_exists( $stylesheet_directory . '/theme.json' ) ) {
$path = $stylesheet_directory . '/theme.json';
} else {
$path = $template_directory . '/theme.json';
}

/** This filter is documented in wp-includes/link-template.php */
$path = apply_filters( 'theme_file_path', $path, 'theme.json' );

$theme_has_support = file_exists( $path );

return $theme_has_support;
}

Expand Down
5 changes: 5 additions & 0 deletions templates/style.php
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,11 @@
.amp-wp-article-featured-image {
margin: 0 0 1em;
}
.amp-wp-article-featured-image img:not(amp-img) {
max-width: 100%;
height: auto;
margin: 0 auto;
}
.amp-wp-article-featured-image amp-img {
margin: 0 auto;
}
Expand Down
28 changes: 25 additions & 3 deletions tests/php/src/Admin/PolyfillsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@ public function test_registration() {

// These should pass in WP < 5.6.
$this->assertTrue( wp_script_is( 'lodash', 'registered' ) );
$this->assertStringContainsString( '_.noConflict();', wp_scripts()->print_inline_script( 'lodash', 'after', false ) );
$this->assertStringContainsString( '_.noConflict();', self::get_inline_script( 'lodash', 'after' ) );

$this->assertTrue( wp_script_is( 'wp-api-fetch', 'registered' ) );
$this->assertStringContainsString( 'createRootURLMiddleware', wp_scripts()->print_inline_script( 'wp-api-fetch', 'after', false ) );
$this->assertStringContainsString( 'createNonceMiddleware', wp_scripts()->print_inline_script( 'wp-api-fetch', 'after', false ) );
$this->assertStringContainsString( 'createRootURLMiddleware', self::get_inline_script( 'wp-api-fetch', 'after' ) );
$this->assertStringContainsString( 'createNonceMiddleware', self::get_inline_script( 'wp-api-fetch', 'after' ) );

$this->assertTrue( wp_script_is( 'wp-hooks', 'registered' ) );
$this->assertTrue( wp_script_is( 'wp-i18n', 'registered' ) );
Expand All @@ -99,4 +99,26 @@ public function test_registration() {

$this->assertTrue( wp_style_is( 'wp-components', 'registered' ) );
}

/**
* Get inline script.
*
* @param string $handle Script handle.
* @param string $position Script position.
*
* @return string
*/
public static function get_inline_script( $handle, $position = 'after' ) {
if ( method_exists( wp_scripts(), 'get_inline_script_tag' ) ) {
$script = wp_scripts()->get_inline_script_tag( $handle, $position );

if ( ! $script ) {
return false;
}

return $script;
} else {
return wp_scripts()->print_inline_script( $handle, $position, false );
}
}
}
13 changes: 7 additions & 6 deletions tests/php/src/Editor/EditorSupportTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use AmpProject\AmpWP\Infrastructure\Service;
use AmpProject\AmpWP\Tests\Helpers\WithBlockEditorSupport;
use AmpProject\AmpWP\Tests\TestCase;
use AmpProject\AmpWP\Tests\Admin\PolyfillsTest;

/** @coversDefaultClass \AmpProject\AmpWP\Editor\EditorSupport */
final class EditorSupportTest extends TestCase {
Expand Down Expand Up @@ -71,15 +72,15 @@ public function test_supports_current_screen( $post_type_uses_block_editor, $pos
/** @covers ::maybe_show_notice() */
public function test_dont_show_notice_if_no_screen_defined() {
$this->instance->maybe_show_notice();
$this->assertFalse( wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false ) );
$this->assertFalse( PolyfillsTest::get_inline_script( 'wp-edit-post', 'after' ) );
}

/** @covers ::maybe_show_notice() */
public function test_dont_show_notice_for_unsupported_post_type() {
$this->setup_environment( true, false );

$this->instance->maybe_show_notice();
$this->assertFalse( wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false ) );
$this->assertFalse( PolyfillsTest::get_inline_script( 'wp-edit-post', 'after' ) );
}

/** @covers ::maybe_show_notice() */
Expand All @@ -92,11 +93,11 @@ public function test_show_notice_for_supported_post_type() {

$this->instance->maybe_show_notice();
if ( $this->instance->is_current_screen_block_editor_for_amp_enabled_post_type() ) {
$this->assertFalse( wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false ) );
$this->assertFalse( PolyfillsTest::get_inline_script( 'wp-edit-post', 'after' ) );
} else {
$this->assertStringContainsString(
'AMP functionality is not available',
wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false )
PolyfillsTest::get_inline_script( 'wp-edit-post', 'after' )
);
}
}
Expand All @@ -108,7 +109,7 @@ public function test_maybe_show_notice_for_unsupported_user() {

$this->instance->maybe_show_notice();

$this->assertFalse( wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false ) );
$this->assertFalse( PolyfillsTest::get_inline_script( 'wp-edit-post', 'after' ) );
}

/** @covers ::maybe_show_notice() */
Expand All @@ -125,7 +126,7 @@ public function test_maybe_show_notice_for_gutenberg_4_9() {
wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) );

$this->instance->maybe_show_notice();
$inline_script = wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false );
$inline_script = PolyfillsTest::get_inline_script( 'wp-edit-post', 'after' );
$this->assertStringContainsString( 'AMP functionality is not available', $inline_script );
}
}
7 changes: 3 additions & 4 deletions tests/php/src/ReaderThemeSupportFeaturesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -483,16 +483,15 @@ public function test_print_theme_support_styles_reader( $is_legacy ) {
$this->assertStringContainsString( '<style id="amp-wp-theme-support-editor-font-sizes">', $output );
$this->assertStringContainsString( 'font-size: clamp(', $output );
$this->assertStringContainsString( '+ ((', $output );
$this->assertStringContainsString( ':root .has-small-font-size { font-size: clamp(0.875rem, 0.875rem + ((1vw - 0.48rem) * 0.24), 1rem); }', $output );
$this->assertStringContainsString( ':root .has-small-font-size { font-size: clamp(0.875rem', $output );
}

// Assert spacing size custom properties.
if ( $this->call_private_method( $this->instance, 'theme_has_theme_json' ) && function_exists( 'wp_get_global_settings' ) ) {
$this->assertStringContainsString( '<style id="amp-wp-theme-support-spacing-sizes-custom-properties">', $output );
$this->assertStringContainsString( ':root {', $output );
$this->assertStringContainsString( '--wp--preset--spacing--30: clamp(1.5rem, 5vw, 2rem);', $output );
$this->assertStringContainsString( 'clamp(1.8rem, 1.8rem + ((1vw - 0.48rem) * 2.885), 3rem);', $output );
$this->assertStringContainsString( '--wp--preset--spacing--80: clamp(7rem, 14vw, 11rem);', $output );
$this->assertStringContainsString( '--wp--preset--spacing--30:', $output );
$this->assertStringContainsString( '--wp--preset--spacing--80:', $output );
$this->assertStringContainsString( '}', $output );
$this->assertStringContainsString( '</style>', $output );
}
Expand Down
3 changes: 3 additions & 0 deletions tests/php/test-amp-gallery-embed-handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,9 @@ public function test__conversion( $source, $expected, $use_legacy_mode = false )
// Remove lazy loading attribute.
$content = preg_replace( '/\s+loading="lazy"/', '', $content );

// Remove fetchpriority attribute.
$content = preg_replace( '/\s+fetchpriority="high"/', '', $content );

$this->assertEquals(
$this->normalize( $expected ),
$this->normalize( $content )
Expand Down

0 comments on commit 3396a0c

Please sign in to comment.