Skip to content

Commit

Permalink
Clone the DOMElementList in add(), and return the clone
Browse files Browse the repository at this point in the history
Thanks to Alain's suggestion,
this makes the DOMElementList immutable.
The $elements property is now public,
but the DOMElements in that were publicly
available through iteration,
so that isn't a huge change.
  • Loading branch information
kienstra committed Nov 24, 2019
1 parent 8de2340 commit 3cdd86f
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 8 deletions.
2 changes: 1 addition & 1 deletion includes/embeds/class-amp-gallery-embed.php
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ public function render( $args ) {
}

$caption = isset( $props['id'] ) ? wp_get_attachment_caption( $props['id'] ) : '';
$images->add( $image, $caption );
$images = $images->add( $image, $caption );
}

$amp_carousel = new Carousel( $dom, $images );
Expand Down
4 changes: 2 additions & 2 deletions includes/sanitizers/class-amp-gallery-block-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,14 @@ public function sanitize() {
// If it's not AMP lightbox, look for links first.
if ( ! $is_amp_lightbox ) {
foreach ( $node->getElementsByTagName( 'a' ) as $element ) {
$images->add( $element, $this->possibly_get_caption_text( $element ) );
$images = $images->add( $element, $this->possibly_get_caption_text( $element ) );
}
}

// If not linking to anything then look for <amp-img>.
if ( 0 === count( $images ) ) {
foreach ( $node->getElementsByTagName( 'amp-img' ) as $element ) {
$images->add( $element, $this->possibly_get_caption_text( $element ) );
$images = $images->add( $element, $this->possibly_get_caption_text( $element ) );
}
}

Expand Down
7 changes: 4 additions & 3 deletions src/Component/DOMElementList.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ final class DOMElementList implements IteratorAggregate, Countable {
*
* @var DOMElement[]
*/
private $elements = [];
public $elements = [];

/**
* Adds an image to the list.
Expand All @@ -35,8 +35,9 @@ final class DOMElementList implements IteratorAggregate, Countable {
* @return self
*/
public function add( DOMElement $element, $caption = '' ) {
$this->elements[] = empty( $caption ) ? $element : new CaptionedSlide( $element, $caption );
return $this;
$cloned_list = clone $this;
$cloned_list->elements[] = empty( $caption ) ? $element : new CaptionedSlide( $element, $caption );
return $cloned_list;
}

/**
Expand Down
4 changes: 2 additions & 2 deletions tests/php/test-dom-element-list.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public function get_dom_element_list_data() {
public function test_dom_element_list_add( $images, $expected_count ) {
$dom_element_list = new DOMElementList();
foreach ( $images as $image ) {
$dom_element_list->add( $image, '' );
$dom_element_list = $dom_element_list->add( $image, '' );
}

$this->assertEquals( $expected_count, $dom_element_list->count() );
Expand All @@ -76,7 +76,7 @@ public function test_dom_element_list_add( $images, $expected_count ) {
public function test_dom_element_list_get_iterator( $images, $expected_count ) {
$dom_element_list = new DOMElementList();
foreach ( $images as $image ) {
$dom_element_list->add( $image, '' );
$dom_element_list = $dom_element_list->add( $image, '' );
}

$iteration_count = 0;
Expand Down

0 comments on commit 3cdd86f

Please sign in to comment.