Skip to content

Commit 8fa1588

Browse files
ediaminediamin
authored
Merge pull request #523 from ampproject/enhance/518-add-amp-audio-ssr-support
Implement SSR for amp-audio
2 parents 517f67b + ead1ba6 commit 8fa1588

File tree

4 files changed

+68
-30
lines changed

4 files changed

+68
-30
lines changed

‎src/Optimizer/Error/CannotRemoveBoilerplate.php‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ final class CannotRemoveBoilerplate implements Error
2222
. 'attribute produced an error: ';
2323
const RENDER_DELAYING_SCRIPT_STRING = 'Cannot remove boilerplate because the document contains a render-delaying '
2424
. 'extension: ';
25-
const AMP_AUDIO_STRING = 'Cannot remove boilerplate because the document contains an extension that '
26-
. 'needs to know the dimensions of the browser: ';
2725
const UNSUPPORTED_LAYOUT_STRING = 'Cannot remove boilerplate because of an unsupported layout: ';
2826

2927
/**
@@ -59,17 +57,6 @@ public static function fromAmpExperiment(Element $element)
5957
return new self(self::RENDER_DELAYING_SCRIPT_STRING . $element->tagName);
6058
}
6159

62-
/**
63-
* Instantiate a CannotRemoveBoilerplate object for an amp-audio element.
64-
*
65-
* @param Element $element amp-audio element.
66-
* @return self
67-
*/
68-
public static function fromAmpAudio(Element $element)
69-
{
70-
return new self(self::AMP_AUDIO_STRING . new ElementDump($element));
71-
}
72-
7360
/**
7461
* Instantiate a CannotRemoveBoilerplate object for an element with an unsupported layout.
7562
*

‎src/Optimizer/Transformer/ServerSideRendering.php‎

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -127,20 +127,18 @@ public function transform(Document $document, ErrorCollection $errors)
127127
}
128128

129129
/*
130-
* amp-experiment is a render delaying extension iff the tag is used in the doc. We check for that here
131-
* rather than checking for the existence of the amp-experiment script in IsRenderDelayingExtension below.
130+
* Server-side rendering of an amp-audio element.
132131
*/
133-
if ($ampElement->tagName === Extension::EXPERIMENT && $this->isAmpExperimentUsed($ampElement)) {
134-
$errors->add(Error\CannotRemoveBoilerplate::fromAmpExperiment($ampElement));
135-
$canRemoveBoilerplate = false;
132+
if ($ampElement->tagName === Extension::AUDIO) {
133+
$this->ssrAmpAudio($document, $ampElement);
136134
}
137135

138136
/*
139-
* amp-audio requires knowing the dimensions of the browser. Do not remove the boilerplate or apply layout
140-
* if amp-audio is present in the document.
137+
* amp-experiment is a render delaying extension iff the tag is used in the doc. We check for that here
138+
* rather than checking for the existence of the amp-experiment script in IsRenderDelayingExtension below.
141139
*/
142-
if ($ampElement->tagName === Extension::AUDIO) {
143-
$errors->add(Error\CannotRemoveBoilerplate::fromAmpAudio($ampElement));
140+
if ($ampElement->tagName === Extension::EXPERIMENT && $this->isAmpExperimentUsed($ampElement)) {
141+
$errors->add(Error\CannotRemoveBoilerplate::fromAmpExperiment($ampElement));
144142
$canRemoveBoilerplate = false;
145143
}
146144

@@ -1060,4 +1058,28 @@ private function isZero($number)
10601058

10611059
return abs($number) < self::FLOATING_POINT_EPSILON;
10621060
}
1061+
1062+
/**
1063+
* Server-side rendering of an amp-audio element.
1064+
*
1065+
* @param Document $document DOM document to apply the transformations to.
1066+
* @param Element $element Element to adapt.
1067+
*/
1068+
private function ssrAmpAudio(Document $document, Element $element)
1069+
{
1070+
// Check if we already have a SSR-ed audio element.
1071+
if ($element->hasChildNodes()) {
1072+
foreach ($element->childNodes as $childNode) {
1073+
if ($childNode instanceof Element && $childNode->tagName === Tag::AUDIO) {
1074+
return;
1075+
}
1076+
}
1077+
}
1078+
1079+
$audio = $document->createElement(Tag::AUDIO);
1080+
$controls = $document->createAttribute(Attribute::CONTROLS);
1081+
1082+
$audio->setAttributeNode($controls);
1083+
$element->appendChild($audio);
1084+
}
10631085
}

‎tests/Optimizer/SpecTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ final class SpecTest extends TestCase
4848
'ReorderHead - preserves_amp_custom_style_order' => 'see https://github.com/ampproject/amp-toolbox/issues/604',
4949

5050
'MinifyHtml - minifies_inline_amp-script' => 'see https://github.com/ampproject/amp-toolbox-php/issues/260',
51+
52+
'ServerSideRendering - does_not_transform_amp_audio' => 'The amp-toolbox needs to implement SSR amp-audio',
5153
];
5254

5355
const CLASS_SKIP_TEST = '__SKIP__';

‎tests/Optimizer/Transformer/ServerSideRenderingTest.php‎

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -132,14 +132,7 @@ public function dataTransform()
132132

133133
'amp-audio' => [
134134
$input('<amp-audio></amp-audio>'),
135-
$expectWithBoilerplate('<amp-audio></amp-audio>'),
136-
[
137-
Error\CannotRemoveBoilerplate::fromAmpAudio(
138-
Document::fromHtmlFragment(
139-
'<amp-audio></amp-audio>'
140-
)->body->firstChild
141-
),
142-
],
135+
$expectWithoutBoilerplate('<amp-audio><audio controls></audio></amp-audio>'),
143136
],
144137

145138
'amp-experiment is non-empty' => [
@@ -326,6 +319,40 @@ public function dataTransform()
326319
$input('<amp-ad type="doubleclick" data-slot="/6355419/Travel" layout="fluid" height="fluid" width="300"></amp-ad>'),
327320
$expectWithoutBoilerplate('<amp-ad class="i-amphtml-layout-fluid i-amphtml-layout-awaiting-size" data-slot="/6355419/Travel" height="fluid" width="300" i-amphtml-layout="fluid" layout="fluid" style="width:100%;height:0;" type="doubleclick"></amp-ad>'),
328321
],
322+
323+
'server side render amp-audio' => [
324+
$input('<amp-audio src="http://example.com/audio.mp3" width="300"></amp-audio>'),
325+
$expectWithoutBoilerplate('<amp-audio src="http://example.com/audio.mp3" width="300"><audio controls></audio></amp-audio>'),
326+
],
327+
328+
'ssr amp-audio appends audio element' => [
329+
$input(
330+
'<amp-audio src="http://example.com/audio.mp3" width="300">'
331+
. '<div fallback="">Your browser doesn’t support HTML5 audio</div>'
332+
. '</amp-audio>'
333+
),
334+
$expectWithoutBoilerplate(
335+
'<amp-audio src="http://example.com/audio.mp3" width="300">'
336+
. '<div fallback="">Your browser doesn’t support HTML5 audio</div>'
337+
. '<audio controls></audio>'
338+
. '</amp-audio>'
339+
),
340+
],
341+
342+
'skip ssr amp-audio if audio child node is present' => [
343+
$input(
344+
'<amp-audio src="http://example.com/audio.mp3" width="300">'
345+
. '<div fallback="">Your browser doesn’t support HTML5 audio</div>'
346+
. '<audio controls src="http://example.com/audio.mp3"></audio>'
347+
. '</amp-audio>'
348+
),
349+
$expectWithoutBoilerplate(
350+
'<amp-audio src="http://example.com/audio.mp3" width="300">'
351+
. '<div fallback="">Your browser doesn’t support HTML5 audio</div>'
352+
. '<audio controls src="http://example.com/audio.mp3"></audio>'
353+
. '</amp-audio>'
354+
),
355+
]
329356
];
330357
}
331358

0 commit comments

Comments
 (0)