Commit c6e1b960 authored by alexpott's avatar alexpott
Browse files

Issue #2679867 by Wim Leers, mr.baileys: BigPipe causes PHP notices with...

Issue #2679867 by Wim Leers, mr.baileys: BigPipe causes PHP notices with JavaScript disabled on error pages
parent 181b4bbb
...@@ -23,14 +23,6 @@ ...@@ -23,14 +23,6 @@
*/ */
class HtmlResponseBigPipeSubscriber implements EventSubscriberInterface { class HtmlResponseBigPipeSubscriber implements EventSubscriberInterface {
/**
* Attribute name of the BigPipe response eligibility test result.
*
* @see onRespondEarly()
* @see onRespond()
*/
const ATTRIBUTE_ELIGIBLE = '_big_pipe_eligible';
/** /**
* The BigPipe service. * The BigPipe service.
* *
...@@ -55,12 +47,8 @@ public function __construct(BigPipeInterface $big_pipe) { ...@@ -55,12 +47,8 @@ public function __construct(BigPipeInterface $big_pipe) {
* The event to process. * The event to process.
*/ */
public function onRespondEarly(FilterResponseEvent $event) { public function onRespondEarly(FilterResponseEvent $event) {
// It does not make sense to have BigPipe responses for subrequests. BigPipe
// is never useful internally in Drupal, only externally towards end users.
$response = $event->getResponse(); $response = $event->getResponse();
$is_eligible = $event->isMasterRequest() && $response instanceof HtmlResponse; if (!$response instanceof HtmlResponse) {
$event->getRequest()->attributes->set(self::ATTRIBUTE_ELIGIBLE, $is_eligible);
if (!$is_eligible) {
return; return;
} }
...@@ -84,13 +72,11 @@ public function onRespondEarly(FilterResponseEvent $event) { ...@@ -84,13 +72,11 @@ public function onRespondEarly(FilterResponseEvent $event) {
* The event to process. * The event to process.
*/ */
public function onRespond(FilterResponseEvent $event) { public function onRespond(FilterResponseEvent $event) {
// Early return if this response was already found to not be eligible. $response = $event->getResponse();
// @see onRespondEarly() if (!$response instanceof HtmlResponse) {
if (!$event->getRequest()->attributes->get(self::ATTRIBUTE_ELIGIBLE)) {
return; return;
} }
$response = $event->getResponse();
$attachments = $response->getAttachments(); $attachments = $response->getAttachments();
// If there are no no-JS BigPipe placeholders, unwrap the scripts_bottom // If there are no no-JS BigPipe placeholders, unwrap the scripts_bottom
......
...@@ -174,6 +174,10 @@ public function testBigPipe() { ...@@ -174,6 +174,10 @@ public function testBigPipe() {
$this->pass('Verifying BigPipe assets are present…', 'Debug'); $this->pass('Verifying BigPipe assets are present…', 'Debug');
$this->assertFalse(empty($this->getDrupalSettings()), 'drupalSettings present.'); $this->assertFalse(empty($this->getDrupalSettings()), 'drupalSettings present.');
$this->assertTrue(in_array('big_pipe/big_pipe', explode(',', $this->getDrupalSettings()['ajaxPageState']['libraries'])), 'BigPipe asset library is present.'); $this->assertTrue(in_array('big_pipe/big_pipe', explode(',', $this->getDrupalSettings()['ajaxPageState']['libraries'])), 'BigPipe asset library is present.');
// Verify that 4xx responses work fine. (4xx responses are handled by
// subrequests to a route pointing to a controller with the desired output.)
$this->drupalGet(Url::fromUri('base:non-existing-path'));
} }
/** /**
...@@ -219,6 +223,10 @@ public function testBigPipeNoJs() { ...@@ -219,6 +223,10 @@ public function testBigPipeNoJs() {
$this->pass('Verifying BigPipe assets are absent…', 'Debug'); $this->pass('Verifying BigPipe assets are absent…', 'Debug');
$this->assertFalse(empty($this->getDrupalSettings()), 'drupalSettings and BigPipe asset library absent.'); $this->assertFalse(empty($this->getDrupalSettings()), 'drupalSettings and BigPipe asset library absent.');
// Verify that 4xx responses work fine. (4xx responses are handled by
// subrequests to a route pointing to a controller with the desired output.)
$this->drupalGet(Url::fromUri('base:non-existing-path'));
} }
protected function assertBigPipeResponseHeadersPresent() { protected function assertBigPipeResponseHeadersPresent() {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment