Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move any content output during shutdown to be injected before closing body tag #1102

Merged
merged 2 commits into from Jun 20, 2018

Conversation

Projects
None yet
2 participants
@westonruter
Copy link
Member

westonruter commented Apr 29, 2018

Plugins like Query Monitor output content at shutdown. This means the output is added after the </html> closing tag. The AMP plugin currently only sanitizes and outputs the contents of the html element. In order to ensure that content after the closing HTML tag is sanitized and incorporated (“embodied”) into the document, we should consider moving the trailing content to be injected before the </body> tag.

  • Add tests.

@westonruter westonruter added this to the v1.0 milestone Apr 29, 2018

@@ -1043,6 +1043,10 @@ public static function prepare_response( $response, $args = array() ) {
1
);
}
// Move anything after </html>, such as Query Monitor output added at shutdown, to be moved before </body>.
$response = preg_replace( '#(</body>.*</html>)(.+)#s', '$2$1', $response );

This comment has been minimized.

Copy link
@westonruter

westonruter Apr 29, 2018

Author Member

Warning: If there is something like <!-- </body></html> --> in the body then this will break. It would be better to use the DOM to move the content since we're parsing it below anyway. We can just move any siblings after the following siblings of $dom->documentElement to be added under the body element.

@westonruter westonruter force-pushed the fix/incorporate-shutdown-output branch from 2f2a315 to c360ddc Jun 20, 2018

@westonruter westonruter requested a review from kienstra Jun 20, 2018

@westonruter westonruter changed the title WIP: Move any content output during shutdown to be injected before closing body tag Move any content output during shutdown to be injected before closing body tag Jun 20, 2018

westonruter added some commits Apr 29, 2018

@westonruter westonruter force-pushed the fix/incorporate-shutdown-output branch from c360ddc to b5948c5 Jun 20, 2018

@kienstra
Copy link
Collaborator

kienstra left a comment

Approved

Hi @westonruter,
This PR looks good. As expected, it moves the #query-monitor element to the end of <body> .

query-monitor-body

@westonruter westonruter merged commit c99e7de into develop Jun 20, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@westonruter westonruter deleted the fix/incorporate-shutdown-output branch Jul 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.