Skip to content

Commit

Permalink
Merge pull request #4482 from ampproject/add/4439-transform-media-siz…
Browse files Browse the repository at this point in the history
…es-heights

Adapt heights, sizes and media attributes for SSR
  • Loading branch information
westonruter committed May 8, 2020
2 parents ca87c3b + 5807091 commit 17f48ff
Show file tree
Hide file tree
Showing 16 changed files with 949 additions and 99 deletions.
14 changes: 7 additions & 7 deletions includes/sanitizers/class-amp-core-theme-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -1638,7 +1638,7 @@ public function wrap_modal_in_lightbox( $args = [] ) {
return;
}

$body_id = AMP_DOM_Utils::get_element_id( $this->dom->body, 'body' );
$body_id = $this->dom->getElementId( $this->dom->body, 'body' );

$open_xpaths = isset( $args['open_button_xpath'] ) ? $args['open_button_xpath'] : [];
$close_xpaths = isset( $args['close_button_xpath'] ) ? $args['close_button_xpath'] : [];
Expand Down Expand Up @@ -1776,7 +1776,7 @@ public function add_twentytwenty_modals() {
}
}

$modal_id = AMP_DOM_Utils::get_element_id( $modal );
$modal_id = $this->dom->getElementId( $modal );

// Add the lightbox itself as a close button xpath as well.
// With twentytwenty compat, the lightbox fills the entire screen, and only an inner wrapper will contain
Expand Down Expand Up @@ -1808,7 +1808,7 @@ public function add_twentytwenty_modals() {
*/
public function add_twentytwenty_toggles() {
$toggles = $this->dom->xpath->query( '//*[ @data-toggle-target ]' );
$body_id = AMP_DOM_Utils::get_element_id( $this->dom->body, 'body' );
$body_id = $this->dom->getElementId( $this->dom->body, 'body' );

if ( false === $toggles || 0 === $toggles->length ) {
return;
Expand All @@ -1821,7 +1821,7 @@ public function add_twentytwenty_toggles() {
*/
foreach ( $toggles as $toggle ) {
$toggle_target = $toggle->getAttribute( 'data-toggle-target' );
$toggle_id = AMP_DOM_Utils::get_element_id( $toggle );
$toggle_id = $this->dom->getElementId( $toggle );

if ( 'next' === $toggle_target ) {
$target_node = $toggle->nextSibling;
Expand All @@ -1847,7 +1847,7 @@ public function add_twentytwenty_toggles() {

$is_sub_menu = AMP_DOM_Utils::has_class( $target_node, 'sub-menu' );
$new_target_node = $is_sub_menu ? $this->get_closest_submenu( $toggle ) : $target_node;
$new_target_id = AMP_DOM_Utils::get_element_id( $new_target_node );
$new_target_id = $this->dom->getElementId( $new_target_node );

$state_string = str_replace( '-', '_', $new_target_id );

Expand All @@ -1869,7 +1869,7 @@ public function add_twentytwenty_toggles() {
// Skip adding the 'active' class on the "Close" button in the primary nav menu.
continue;
}
$target_toggle_id = AMP_DOM_Utils::get_element_id( $target_toggle );
$target_toggle_id = $this->dom->getElementId( $target_toggle );
AMP_DOM_Utils::add_amp_action( $toggle, 'tap', "{$target_toggle_id}.toggleClass(class='active')" );
}
}
Expand All @@ -1888,7 +1888,7 @@ public function add_twentytwenty_toggles() {
$focus_element = $this->dom->xpath->query( $focus_xpath )->item( 0 );

if ( $focus_element instanceof DOMElement ) {
$focus_element_id = AMP_DOM_Utils::get_element_id( $focus_element );
$focus_element_id = $this->dom->getElementId( $focus_element );
AMP_DOM_Utils::add_amp_action( $toggle, 'tap', "{$focus_element_id}.focus" );
}
}
Expand Down
29 changes: 10 additions & 19 deletions includes/utils/class-amp-dom-utils.php
Original file line number Diff line number Diff line change
Expand Up @@ -365,31 +365,22 @@ public static function has_class( DOMElement $element, $class ) {
* If the element does not have an ID, create one first.
*
* @since 1.4.0
* @since 1.5.1 Deprecated for AmpProject\Dom\Document::getElementId()
*
* @deprecated Use AmpProject\Dom\Document::getElementId() instead.
*
* @param DOMElement $element Element to get the ID for.
* @param string $prefix Optional. Defaults to '_amp_wp_id_'.
* @param string $prefix Optional. Defaults to 'amp-wp-id'.
* @return string ID to use.
*/
public static function get_element_id( $element, $prefix = 'amp-wp-id' ) {
static $index_counter = [];

if ( $element->hasAttribute( 'id' ) ) {
return $element->getAttribute( 'id' );
}

if ( ! array_key_exists( $prefix, $index_counter ) ) {
$index_counter[ $prefix ] = 2;
$element->setAttribute( 'id', $prefix );

return $prefix;
}

$id = "{$prefix}-{$index_counter[ $prefix ]}";
$index_counter[ $prefix ] ++;

$element->setAttribute( 'id', $id );
_deprecated_function(
'AMP_DOM_Utils::get_element_id',
'1.5.1',
'Use AmpProject\Amp\Dom\Document::getElementId() instead'
);

return $id;
return Document::fromNode( $element )->getElementId( $element, $prefix );
}

/**
Expand Down
4 changes: 2 additions & 2 deletions lib/common/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@
"ext-dom": "*",
"ext-iconv": "*",
"ext-json": "*",
"ext-libxml": "*"
"ext-libxml": "*",
"php-parallel-lint/php-parallel-lint": "^1.2"
},
"require-dev": {
"civicrm/composer-downloads-plugin": "^2.1",
"jakub-onderka/php-parallel-lint": "^1.0",
"roave/security-advisories": "dev-master",
"squizlabs/php_codesniffer": "^3"
},
Expand Down
1 change: 1 addition & 0 deletions lib/common/src/Attribute.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ interface Attribute
const PROFILE = 'profile';
const REL = 'rel';
const ROLE = 'role';
const SRCSET = 'srcset';
const SIZES = 'sizes';
const STYLE = 'style';
const SRC = 'src';
Expand Down
79 changes: 79 additions & 0 deletions lib/common/src/Dom/CssByteCountCalculator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
<?php

namespace AmpProject\Dom;

use DOMNodeList;

/**
* Class AmpProject\Dom\CssByteCountCalculator.
*
* Calculates the total byte count of CSS styles in a given document.
*
* This can be used to check against the allowed limit of 75kB that AMP enforces.
*
* @package ampproject/common
*/
final class CssByteCountCalculator
{

/**
* XPath query to fetch the <style amp-custom> tag, relative to the <head> node.
*
* @var string
*/
const AMP_CUSTOM_STYLE_TAG_XPATH = './/style[@amp-custom]';

/**
* XPath query to fetch the inline style attributes, relative to the <body> node.
*
* @var string
*/
const INLINE_STYLE_ATTRIBUTES_XPATH = './/@style';

/**
* Document to calculate the total byte count for.
*
* @var Document
*/
private $document;

/**
* Instantiate a CssByteCountCalculator object.
*
* @param Document $document Document to calculate the total byte count for.
*/
public function __construct(Document $document)
{
$this->document = $document;
}

/**
* Calculate the byte count for the provided document.
*
* @return int Total byte count of CSS styles in the document.
*/
public function calculate()
{
$ampCustomStyle = $this->document->xpath->query(self::AMP_CUSTOM_STYLE_TAG_XPATH, $this->document->head);
$inlineStyles = $this->document->xpath->query(self::INLINE_STYLE_ATTRIBUTES_XPATH, $this->document->body);

return $this->calculateForNodeList($ampCustomStyle) + $this->calculateForNodeList($inlineStyles);
}

/**
* Calculate the byte count of CSS styles for a given node list.
*
* @param DOMNodeList $nodeList Node list to calculate the byte count of CSS styles for.
* @return int Byte count of CSS styles for the given node list.
*/
private function calculateForNodeList(DOMNodeList $nodeList)
{
$byteCount = 0;

foreach ($nodeList as $node) {
$byteCount += strlen($node->textContent);
}

return $byteCount;
}
}
44 changes: 43 additions & 1 deletion lib/common/src/Dom/Document.php
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,15 @@ final class Document extends DOMDocument
*/
private $usedAmpEmoji;

/**
* Store the current index by prefix.
*
* This is used to generate unique-per-prefix IDs.
*
* @var int[]
*/
private $indexCounter = [];

/**
* Creates a new AmpProject\Dom\Document object
*
Expand Down Expand Up @@ -314,7 +323,7 @@ public static function fromNode(DOMNode &$node)
// We replace the $node by reference, to make sure the next lines of code will
// work as expected with the new document.
// Otherwise $dom and $node would refer to two different DOMDocuments.
$node = $dom->importNode($root->documentElement ?: $root, true);
$node = $dom->importNode($node, true);
$dom->appendChild($node);

$dom->hasInitialAmpDevMode = $dom->documentElement->hasAttribute(DevMode::DEV_MODE_ATTRIBUTE);
Expand Down Expand Up @@ -1474,6 +1483,39 @@ public function isValidHeadNode(DOMNode $node)
);
}

/**
* Get the ID for an element.
*
* If the element does not have an ID, create one first.
*
* @param DOMElement $element Element to get the ID for.
* @param string $prefix Optional. The prefix to use (should not have a trailing dash). Defaults to 'i-amp-id'.
* @return string ID to use.
*/
public function getElementId(DOMElement $element, $prefix = 'i-amp-id')
{
if ($element->hasAttribute('id')) {
return $element->getAttribute('id');
}

if (array_key_exists($prefix, $this->indexCounter)) {
++$this->indexCounter[$prefix];
$id = "{$prefix}-{$this->indexCounter[ $prefix ]}";
} else {
$id = $prefix;
$this->indexCounter[$prefix] = 1;
}

while ($this->getElementById($id) instanceof DOMElement) {
++$this->indexCounter[$prefix];
$id = "{$prefix}-{$this->indexCounter[ $prefix ]}";
}

$element->setAttribute('id', $id);

return $id;
}

/**
* Determine whether `data-ampdevmode` was initially set on the document element.
*
Expand Down
58 changes: 58 additions & 0 deletions lib/common/tests/Dom/CssByteCountCalculatorTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php

namespace AmpProject\Common;

use AmpProject\Dom\CssByteCountCalculator;
use AmpProject\Dom\Document;
use PHPUnit\Framework\TestCase;

/**
* Tests for AmpProject\Dom\CssByteCountCalculator.
*
* @covers CssByteCountCalculator
* @package ampproject/common
*/
class CssByteCountCalculatorTest extends TestCase
{

/**
* Data provider for testing the calculate() method.
*
* @return array Testing data.
*/
public function dataCalculate()
{
return [
'amp_custom_style_tag' => [
'<html><head><style amp-custom>12345</style>', 5,
],
'one_inline_style_attribute' => [
'<html><body><div style="12345"></div></body></html>', 5,
],
'multiple_inline_style_attributes' => [
'<html><body><div style="1234"></div><div style="567"><div style="89"></body></html>', 9,
],
'amp_custom_style_tag_and_multiple_inline_style_attributes' => [
'<html><head><style amp-custom>12345</style></head><body><div style="1234"></div><div style="567"><div style="89"></body></html>', 14,
],
'amp_custom_style_tag_outside_head' => [
'<html><head><style amp-custom>12345</style></head><body><style amp-custom>123</style></body></html>', 5,
],
'multibyte_chars_are_counted_in_bytes_not_chars' => [
'<html><head><style amp-custom>Iñtërnâtiônàlizætiøn</style></head><body><div style="Iñtërnâtiônàlizætiøn"></div></body></html>', 54,
],
];
}

/**
* Test the calculate() method.
*
* @dataProvider dataCalculate
* @covers CssByteCountCalculator::calculate()
*/
public function testCalculate($html, $expected)
{
$document = Document::fromHtml($html);
$this->assertEquals($expected, (new CssByteCountCalculator($document))->calculate());
}
}
Loading

0 comments on commit 17f48ff

Please sign in to comment.