Misnamed template can cause fatal error in themes that do not extend Stable

Here is a patch just with the renaming.

Should we add a test for this as well, or it's not worth on this kind of obvious typo?

Looks good to me. Tested it with the stark theme. Also added a test for this.

Yes, the test does not fail without clearing the cache. Which makes sense since it will use the cached result for the page otherwise.

Excellent. Thank you.

Showed this to @alexpott in Slack and he wondered if there is any way we can have the test do something a little less heavy-handed than flushing call caches. Maybe just clearing the render cache will do the trick here?

+++ b/core/modules/media/tests/src/FunctionalJavascript/MediaSourceOEmbedVideoTest.php
@@ -136,6 +136,13 @@ public function testMediaOEmbedVideoSource() {
+    drupal_flush_all_caches();

So the test passes without this so we need to have asserts that would cause the test to fail. I.e some asserts either side of this that can determine which template was used. Also let's call $this->resetAll(); so the test container is correctly updated.

In the end it was the cached http response that we should clear. Changed the patch to just invalidate that.

This looks good to me.

+++ b/core/modules/media/tests/src/FunctionalJavascript/MediaSourceOEmbedVideoTest.php
@@ -2,6 +2,7 @@
 use Drupal\media_test_oembed\Controller\ResourceController;

@@ -136,6 +137,13 @@ public function testMediaOEmbedVideoSource() {
+    Cache::invalidateTags(['http_response']);

This points to an bigger issue. Patch incoming.

The iframe response should have the rendered cache tag because it is something produced by the render system but because it is a special response it's not feed through \Drupal\Core\Render\MainContent\HtmlRenderer::renderResponse() so it doesn't get this cache tag. If it did then \Drupal\system\EventSubscriber\ConfigCacheTag::onSave() would clear it automatically for us. So we need to change \Drupal\media\Controller\OEmbedIframeController to add this cache tag. Looking at that code it is apparent that we need to merge the final render array with the response cache data so we get anything added by the render system. Also this allows us to add the missing cache tags.

Also I've improved the test to actually assert on which templates are being used by turning on twig debug. This means the test fails even if the file is correctly named but the cache tag is not added by OEmbedIframeController.

Looks good to me! Probably need someone else to RTBC though.

+++ b/core/modules/media/tests/src/FunctionalJavascript/MediaSourceOEmbedVideoTest.php
@@ -30,6 +31,18 @@ protected function setUp() {
+    // Enable twig debugging to make testing template usage easy.
+    $parameters = $container->getParameter('twig.config');
+    $parameters['debug'] = TRUE;
+    $this->setContainerParameter('twig.config', $parameters);

@@ -135,6 +148,16 @@ public function testMediaOEmbedVideoSource() {
+    $this->assertRaw('core/themes/stable/templates/content/media-oembed-iframe.html.twig');
+    $this->assertNoRaw('core/modules/media/templates/media-oembed-iframe.html.twig');

This is pretty neat. I like it

Patch looks good to me.