diff --git a/includes/sanitizers/class-amp-core-theme-sanitizer.php b/includes/sanitizers/class-amp-core-theme-sanitizer.php index 0cc32de0ec0..89eea469975 100644 --- a/includes/sanitizers/class-amp-core-theme-sanitizer.php +++ b/includes/sanitizers/class-amp-core-theme-sanitizer.php @@ -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'] : []; @@ -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 @@ -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; @@ -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; @@ -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 ); @@ -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')" ); } } @@ -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" ); } } diff --git a/includes/utils/class-amp-dom-utils.php b/includes/utils/class-amp-dom-utils.php index ffa2aff6085..c7eadeaf234 100644 --- a/includes/utils/class-amp-dom-utils.php +++ b/includes/utils/class-amp-dom-utils.php @@ -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 ); } /** diff --git a/lib/common/composer.json b/lib/common/composer.json index 5509862caa9..d92993142eb 100644 --- a/lib/common/composer.json +++ b/lib/common/composer.json @@ -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" }, diff --git a/lib/common/src/Attribute.php b/lib/common/src/Attribute.php index 03c6f57a54d..9555b5d7ed6 100644 --- a/lib/common/src/Attribute.php +++ b/lib/common/src/Attribute.php @@ -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'; diff --git a/lib/common/src/Dom/CssByteCountCalculator.php b/lib/common/src/Dom/CssByteCountCalculator.php new file mode 100644 index 00000000000..89accc786f8 --- /dev/null +++ b/lib/common/src/Dom/CssByteCountCalculator.php @@ -0,0 +1,79 @@ + tag, relative to the
node. + * + * @var string + */ + const AMP_CUSTOM_STYLE_TAG_XPATH = './/style[@amp-custom]'; + + /** + * XPath query to fetch the inline style attributes, relative to the 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; + } +} diff --git a/lib/common/src/Dom/Document.php b/lib/common/src/Dom/Document.php index 3e0a64a2526..428e9c6f472 100644 --- a/lib/common/src/Dom/Document.php +++ b/lib/common/src/Dom/Document.php @@ -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 * @@ -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); @@ -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. * diff --git a/lib/common/tests/Dom/CssByteCountCalculatorTest.php b/lib/common/tests/Dom/CssByteCountCalculatorTest.php new file mode 100644 index 00000000000..a2b2e672de7 --- /dev/null +++ b/lib/common/tests/Dom/CssByteCountCalculatorTest.php @@ -0,0 +1,58 @@ + [ + '', 5, + ], + 'one_inline_style_attribute' => [ + '', 5, + ], + 'multiple_inline_style_attributes' => [ + '