Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Hey, great module! I've used it to implement AMP pages on a site I'm developing.
One miss with this module is that it does not allow meta tags from the metatag module to be placed on AMP pages. This is huge for the usefulness of this module I feel, since AMP pages are meant to greatly enhance your search presence.
To tackle this, I started investigating how metatag includes its tags. It uses #attached, which gets processed through core's render pipeline. AMP pages, however, remove the #attached part of pages entirely in favor of building their own page render array in the controller. While this is good since it makes nice AMP pages without "extra" libraries/css creeping in, I feel that it is missing the benefits of using the render pipeline properly.
As an aside, I feel like AMP is a different response type altogether, much like how 'html', 'ajax', etc are differentiated in core. These AMP pages are still being treated as the HTML type, and providing core HTML responses. Extending HTML seems like it would be more appropriate.
I have built a separate module with some small mods to simple_amp, to get meta tags working with the knowledge above. For longevity I don't feel like this is the best approach for my project since this module will see future changes, so I'm going to work my separate module into simple_amp's codebase here to provide a larger patch. It seems like this patch would position simple_amp to use the render pipeline and allow for metatags to be put into its pages nicely.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 2943339_schema_metatg_support_fix_drupal_errors.patch | 1.31 KB | Lilit_Ghazaryan |
| #9 | interdiff_7-8.txt | 5.01 KB | Lilit_Ghazaryan |
| #8 | 2943339_simple-amp_support_metatag_render_pipeline_update_with_supporting_schema_metatag.patch | 43.92 KB | Lilit_Ghazaryan |
| #7 | 2943339_simple-amp_support_metatag_render_pipeline_update_6.patch | 41.49 KB | Anonymous (not verified) |
| #5 | 2943339_simple-amp_support_metatag_render_pipeline_update_5.patch | 41.54 KB | Anonymous (not verified) |
Comments
Comment #1
Anonymous (not verified) CreditAttribution: Anonymous commentedvilepickle created an issue. See original summary.
Comment #2
Anonymous (not verified) CreditAttribution: Anonymous commentedFirst pass, feedback/improvements to patch welcome.
To test this patch:
Install simple_amp with metatag module. If you have meta tags set up for your amp node types, they should show up in the HEAD of the AMP page.
Comment #3
Anonymous (not verified) CreditAttribution: Anonymous commentedIncluded the fix from #2915890 by mistake. Updated patch
Comment #4
Anonymous (not verified) CreditAttribution: Anonymous commentedFixing case with special characters in URL weren't finding the entity
Comment #5
Anonymous (not verified) CreditAttribution: Anonymous commentedAdding a null check for the attachment processor when variables might be null.
Comment #6
minnur CreditAttribution: minnur at Chapter Three commentedThanks for your help! I will review and apply the patch when I will have some capacity.
Comment #7
Anonymous (not verified) CreditAttribution: Anonymous commentedCool, sounds great! And oops, I looked at my last patch and I hit the wrong commit with the git diff, so it removed all the new stuff...
This one should fix.
Comment #8
Lilit_Ghazaryan CreditAttribution: Lilit_Ghazaryan at EPAM Systems, Wolters Kluwer commentedModified #7 patch for adding metatag and schema metatag in amp pages
Comment #9
Lilit_Ghazaryan CreditAttribution: Lilit_Ghazaryan at EPAM Systems, Wolters Kluwer commentedComment #11
Lilit_Ghazaryan CreditAttribution: Lilit_Ghazaryan at EPAM Systems, Wolters Kluwer commentedafter applying patch I found errors, and this patch is fixing errors
Notice: Undefined index: schema_metatag in Drupal\simple_amp\Render\AmpResponseAttachmentsProcessor->processAmpHtmlHead() (line 235 of modules/contrib/simple_amp/src/Render/AmpResponseAttachmentsProcessor.php).
Drupal\simple_amp\Render\AmpResponseAttachmentsProcessor->processAmpHtmlHead(Array) (Line: 130)
Drupal\simple_amp\Render\AmpResponseAttachmentsProcessor->processAmpAttachments(Object) (Line: 82)
Drupal\simple_amp\EventSubscriber\AmpResponseSubscriber->onRespond(Object, 'kernel.response', Object)
call_user_func(Array, Object, 'kernel.response', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.response', Object) (Line: 191)
Symfony\Component\HttpKernel\HttpKernel->filterResponse(Object, Object, 1) (Line: 173)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 693)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
User error: "1" is an invalid render array key in Drupal\Core\Render\Element::children() (line 97 of core/lib/Drupal/Core/Render/Element.php).
Drupal\Core\Render\Element::children(Array, 1) (Line: 408)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 56)
Drupal\simple_amp\Render\BaseAmpRenderer->doRender(Array) (Line: 450)
Drupal\Core\Render\Renderer->doRender(Array, 1) (Line: 56)
Drupal\simple_amp\Render\BaseAmpRenderer->doRender(Array, 1) (Line: 195)
Drupal\Core\Render\Renderer->render(Array, 1) (Line: 151)
Drupal\Core\Render\Renderer->Drupal\Core\Render\{closure}() (Line: 582)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 152)
Drupal\Core\Render\Renderer->renderPlain(Array) (Line: 214)
Drupal\simple_amp\Render\AmpResponseAttachmentsProcessor->renderAmpResponseAttachmentPlaceholders(Object, Array, Array) (Line: 136)
Drupal\simple_amp\Render\AmpResponseAttachmentsProcessor->processAmpAttachments(Object) (Line: 82)
Drupal\simple_amp\EventSubscriber\AmpResponseSubscriber->onRespond(Object, 'kernel.response', Object)
call_user_func(Array, Object, 'kernel.response', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.response', Object) (Line: 191)
Symfony\Component\HttpKernel\HttpKernel->filterResponse(Object, Object, 1) (Line: 173)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 693)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
Comment #12
slasher13Please commit patch from #11, too.
Comment #13
minnur CreditAttribution: minnur at Chapter Three commented@Lilit_Ghazaryan thanks, I will review and commit shortly.
Comment #15
minnur CreditAttribution: minnur at Chapter Three commentedThanks all the work, I applied the patch. I will review few more issues and make a new release.