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

Add support for comments #871

Closed
wants to merge 39 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
7ec60e4
Make add `get_dom` method to AMP_DOM_Utils
DavidCramer Jan 17, 2018
eed214f
add amp_form_sanitizer
DavidCramer Jan 17, 2018
4067b59
add form sanitisation on output buuffer
DavidCramer Jan 17, 2018
a8d2ad3
merge develop in
DavidCramer Jan 18, 2018
98874ba
Merge develop
DavidCramer Jan 19, 2018
4f6dcd6
add comments REST endpoints
DavidCramer Jan 19, 2018
317f90f
Add AMP Comments Walker
DavidCramer Jan 19, 2018
451db91
Add comments template overrides and methods
DavidCramer Jan 19, 2018
0541686
add comment sanitizer
DavidCramer Jan 19, 2018
2c8575e
Make walker add the HTML to reduce load in the sanitiser.
DavidCramer Jan 19, 2018
ac0cc53
its a filter, not an action.
DavidCramer Jan 19, 2018
2741fdf
Merge develop
DavidCramer Jan 22, 2018
ebffaae
add form sanitisation changes
DavidCramer Jan 22, 2018
9790be3
make action setting more sane
DavidCramer Jan 22, 2018
b425b19
Add sanitisation and component_type method
DavidCramer Jan 22, 2018
b7c5963
optimize comments endpoint
DavidCramer Jan 23, 2018
94aee02
wp_unslash request_uri for action attribute.
DavidCramer Jan 23, 2018
d0fc6c5
cleaner comments template generation
DavidCramer Jan 23, 2018
6b660b3
merge develop
DavidCramer Jan 23, 2018
69d1342
cleanup formatting
DavidCramer Jan 24, 2018
4200d9e
output comment template on same comments pass
DavidCramer Jan 24, 2018
366bef3
cleanup comments sanitiser and add url template string filtering
DavidCramer Jan 24, 2018
7b63836
add comment template url string filters
DavidCramer Jan 24, 2018
8b289c1
remove unnecessary duplicated `wp_list_comments` pass
DavidCramer Jan 24, 2018
15d327d
Hook into AMP xhr-submissions to handle headers and JSON responses.
DavidCramer Jan 24, 2018
561189b
add amp-list fallback for comments.
DavidCramer Jan 24, 2018
a1bf8d3
add comment form success and error handlers and wrapper code.
DavidCramer Jan 24, 2018
b40e932
lowercase `post` k, thanks.
DavidCramer Jan 24, 2018
dc1f9c3
Merge develop
DavidCramer Jan 24, 2018
187e136
fix formatting fail.
DavidCramer Jan 24, 2018
82e8021
if no template, move on.
DavidCramer Jan 25, 2018
d4fd8bc
No need to check for the `amp_comments` tag anymore.
DavidCramer Jan 25, 2018
bbabbd7
add tests for form and comment sanitisation.
DavidCramer Jan 25, 2018
0fd1ab5
add docs to tests
DavidCramer Jan 25, 2018
9f99abe
No need to get images again.
DavidCramer Jan 25, 2018
9216e4e
check has images before converting urls
DavidCramer Jan 25, 2018
fafa679
fix phpdoc
DavidCramer Jan 25, 2018
9536794
Merge branch 'develop' of https://github.com/Automattic/amp-wp into a…
westonruter Jan 25, 2018
74ae3b5
Prevent saveHTML from munging amp-mustache curly braces via URL-encoding
westonruter Jan 26, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions includes/amp-helper-functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,8 @@ function amp_get_content_sanitizers( $post = null ) {
'AMP_Video_Sanitizer' => array(),
'AMP_Audio_Sanitizer' => array(),
'AMP_Playbuzz_Sanitizer' => array(),
'AMP_Form_Sanitizer' => array(),
'AMP_Comments_Sanitizer' => array(),
'AMP_Iframe_Sanitizer' => array(
'add_placeholder' => true,
),
Expand Down
20 changes: 19 additions & 1 deletion includes/class-amp-theme-support.php
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,23 @@ public static function filter_the_content( $content ) {
return $sanitized_content;
}

/**
* Determine the type of component script.
*
* @param string $component The component name.
* @return string the type of component.
*/
public static function get_component_type( $component ) {
$component_types = apply_filters( 'amp_component_types', array(
'amp-mustache' => 'template',
) );

if ( ! empty( $component_types[ $component ] ) ) {
return $component_types[ $component ];
}
return 'element';
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@westonruter component scripts are have custom-element in the script tags, however amp-mustache used for amp-list does not, instead it has custom-template. see https://www.ampproject.org/docs/reference/components/amp-mustache
It looks like this is the only component that's different, however I separated it to its own method in case the spec changes, or additionals are added in future.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think get_component_type would be needed if we sanitize the entire body, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@westonruter I think we still do, since this is done after the sanitisation and the scripts are placed in. All the scripts are custom-element but the template is custom-template.

/**
* Determine required AMP scripts.
*
Expand Down Expand Up @@ -571,7 +588,8 @@ public static function get_amp_component_scripts( $html ) {
$scripts = '';
foreach ( $amp_scripts as $amp_script_component => $amp_script_source ) {
$scripts .= sprintf(
'<script async custom-element="%s" src="%s"></script>', // phpcs:ignore WordPress.WP.EnqueuedResources, WordPress.XSS.EscapeOutput.OutputNotEscaped
'<script async custom-%s="%s" src="%s"></script>', // phpcs:ignore WordPress.WP.EnqueuedResources, WordPress.XSS.EscapeOutput.OutputNotEscaped
self::get_component_type( $amp_script_component ),
$amp_script_component,
$amp_script_source
);
Expand Down