Skip to content

Commit e45877b

Browse files
westonruterwestonruter
authored
Merge pull request #4401 from ampproject/add/validation-error-titles-to-block-warnings
Reuse validation error titles from validated URL screen in block warning notice
2 parents ab65349 + 3c2ae84 commit e45877b

File tree

8 files changed

+33
-99
lines changed

8 files changed

+33
-99
lines changed

‎assets/src/block-validation/components/higher-order/with-validation-error-notice/edit.css‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
.amp-block-validation-errors {
1+
.amp-block-validation-errors,
2+
.amp-block-validation-errors * {
23
font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, Oxygen-Sans, Ubuntu, Cantarell, "Helvetica Neue", sans-serif;
34
font-size: 13px;
45
line-height: 1.5;

‎assets/src/block-validation/components/validation-error-message/index.js‎

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,42 +7,25 @@ import { ReactElement } from 'react';
77
/**
88
* WordPress dependencies
99
*/
10-
import { __, sprintf } from '@wordpress/i18n';
10+
import { __ } from '@wordpress/i18n';
1111

1212
/**
1313
* Get message for validation error.
1414
*
15-
* @param {Object} props Component props.
15+
* @param {Object} props Component props.
16+
* @param {?string} props.title Title for error (with HTML) as provided by \AMP_Validation_Error_Taxonomy::get_error_title_from_code().
1617
* @param {?string} props.code Error code.
17-
* @param {?string} props.node_name Node name.
18-
* @param {?string} props.parent_name Parent node name.
1918
* @param {?string|ReactElement} props.message Error message.
2019
*
2120
* @return {ReactElement} Validation error message.
2221
*/
23-
const ValidationErrorMessage = ( { message, code, node_name: nodeName, parent_name: parentName } ) => {
22+
const ValidationErrorMessage = ( { title, message, code } ) => {
2423
if ( message ) {
25-
return message;
24+
return message; // @todo It doesn't appear this is ever set?
2625
}
2726

28-
if ( 'DISALLOWED_TAG' === code && nodeName ) { // @todo Needs to be fleshed out.
29-
return (
30-
<>
31-
{ __( 'Invalid element: ', 'amp' ) }
32-
<code>
33-
{ nodeName }
34-
</code>
35-
</>
36-
);
37-
} else if ( 'DISALLOWED_ATTR' === code && nodeName ) { // @todo Needs to be fleshed out.
38-
return (
39-
<>
40-
{ __( 'Invalid attribute: ', 'amp' ) }
41-
<code>
42-
{ parentName ? sprintf( '%s[%s]', parentName, nodeName ) : nodeName }
43-
</code>
44-
</>
45-
);
27+
if ( title ) {
28+
return <span dangerouslySetInnerHTML={ { __html: title } } />
4629
}
4730

4831
return (
@@ -57,9 +40,8 @@ const ValidationErrorMessage = ( { message, code, node_name: nodeName, parent_na
5740

5841
ValidationErrorMessage.propTypes = {
5942
message: PropTypes.string,
43+
title: PropTypes.string,
6044
code: PropTypes.string,
61-
node_name: PropTypes.string,
62-
parent_name: PropTypes.string,
6345
};
6446

6547
export default ValidationErrorMessage;

‎assets/src/block-validation/components/validation-error-message/test/__snapshots__/index.js.snap‎

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,31 +9,13 @@ Array [
99
]
1010
`;
1111

12-
exports[`ValidationErrorMessage renders an error for an invalid attribute 1`] = `
13-
Array [
14-
"Invalid attribute: ",
15-
<code>
16-
bar
17-
</code>,
18-
]
19-
`;
20-
21-
exports[`ValidationErrorMessage renders an error for an invalid attribute with parent node 1`] = `
22-
Array [
23-
"Invalid attribute: ",
12+
exports[`ValidationErrorMessage renders an error with a title 1`] = `
13+
<span>
14+
Invalid attribute
2415
<code>
25-
baz[bar]
26-
</code>,
27-
]
28-
`;
29-
30-
exports[`ValidationErrorMessage renders an error for an invalid element 1`] = `
31-
Array [
32-
"Invalid element: ",
33-
<code>
34-
foo
35-
</code>,
36-
]
16+
onclick
17+
</code>
18+
</span>
3719
`;
3820

3921
exports[`ValidationErrorMessage renders an error for an unknown error code 1`] = `

‎assets/src/block-validation/components/validation-error-message/test/index.js‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,8 @@ describe( 'ValidationErrorMessage', () => {
1414
expect( errorMessage ).toMatchSnapshot();
1515
} );
1616

17-
it( 'renders an error for an invalid element', () => {
18-
const errorMessage = render( <ValidationErrorMessage code="DISALLOWED_TAG" node_name="foo" /> );
19-
expect( errorMessage ).toMatchSnapshot();
20-
} );
21-
22-
it( 'renders an error for an invalid attribute', () => {
23-
const errorMessage = render( <ValidationErrorMessage code="DISALLOWED_ATTR" node_name="bar" /> );
24-
expect( errorMessage ).toMatchSnapshot();
25-
} );
26-
27-
it( 'renders an error for an invalid attribute with parent node', () => {
28-
const errorMessage = render( <ValidationErrorMessage code="DISALLOWED_ATTR" node_name="bar" parent_name="baz" /> );
17+
it( 'renders an error with a title', () => {
18+
const errorMessage = render( <ValidationErrorMessage title="Invalid attribute <code>onclick</code>" /> );
2919
expect( errorMessage ).toMatchSnapshot();
3020
} );
3121

‎assets/src/block-validation/helpers/index.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,15 @@ export const updateValidationErrors = () => {
7171
/**
7272
* @param {Object} result Validation error result.
7373
* @param {Object} result.error Error object.
74+
* @param {string} result.title Error title.
7475
* @param {boolean} result.forced Whether sanitization was forced.
7576
* @param {boolean} result.sanitized Whether the error has been sanitized or not.
7677
* @param {number} result.status Validation error status.
7778
* @param {number} result.term_status Error status.
7879
*/
7980
const validationErrors = ampValidity.results.filter( ( result ) => {
8081
return result.term_status !== VALIDATION_ERROR_ACK_ACCEPTED_STATUS; // If not accepted by the user.
81-
} ).map( ( { error, status } ) => ( { ...error, status } ) ); // Merge status into error since needed in maybeDisplayNotice.
82+
} ).map( ( { error, status, title } ) => ( { ...error, status, title } ) ); // Merge status into error since needed in maybeDisplayNotice.
8283

8384
if ( isEqual( validationErrors, previousValidationErrors ) ) {
8485
return;

‎includes/validation/class-amp-validation-manager.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -814,6 +814,7 @@ public static function get_amp_validity_rest_field( $post_data, $field_name, $re
814814
foreach ( AMP_Validated_URL_Post_Type::get_invalid_url_validation_errors( $validation_status_post ) as $result ) {
815815
$field['results'][] = [
816816
'sanitized' => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACK_ACCEPTED_STATUS === $result['status'],
817+
'title' => AMP_Validation_Error_Taxonomy::get_error_title_from_code( $result['data'] ),
817818
'error' => $result['data'],
818819
'status' => $result['status'],
819820
'term_status' => $result['term_status'],

‎tests/php/test-amp-facebook-embed.php‎

Lines changed: 11 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,7 @@ public function get_raw_embed_dataset() {
7777
'https://www.facebook.com/zuck/posts/10102593740125791' . PHP_EOL,
7878
'
7979
<amp-facebook width="500" height="400" data-href="https://www.facebook.com/zuck/posts/10102593740125791" data-embed-as="post" layout="responsive">
80-
<blockquote cite="https://www.facebook.com/zuck/posts/10102593740125791" class="fb-xfbml-parse-ignore" fallback="">
81-
<p>February 4 is Facebook’s 12th birthday!</p>
82-
<p>Our anniversary has a lot of meaning to me as an opportunity to reflect on how…</p>
83-
<p>Posted by <a href="https://www.facebook.com/zuck">Mark Zuckerberg</a> on <a href="https://www.facebook.com/zuck/posts/10102593740125791">Tuesday, January 12, 2016</a></p>
84-
</blockquote>
80+
<blockquote cite="https://www.facebook.com/zuck/posts/10102593740125791" class="fb-xfbml-parse-ignore" fallback=""><!--blockquote_contents--></blockquote>
8581
</amp-facebook>
8682
' . PHP_EOL,
8783
],
@@ -90,9 +86,7 @@ public function get_raw_embed_dataset() {
9086
'https://www.facebook.com/notes/facebook-engineering/under-the-hood-the-javascript-sdk-truly-asynchronous-loading/10151176218703920/' . PHP_EOL,
9187
'
9288
<amp-facebook width="500" height="400" data-href="https://www.facebook.com/notes/facebook-engineering/under-the-hood-the-javascript-sdk-truly-asynchronous-loading/10151176218703920/" data-embed-as="post" layout="responsive">
93-
<blockquote cite="https://www.facebook.com/notes/facebook-engineering/under-the-hood-the-javascript-sdk-truly-asynchronous-loading/10151176218703920/" class="fb-xfbml-parse-ignore" fallback="">
94-
<p>Posted by <a href="https://www.facebook.com/Engineering/">Facebook Engineering</a> on <a href="https://www.facebook.com/notes/facebook-engineering/under-the-hood-the-javascript-sdk-truly-asynchronous-loading/10151176218703920/">Saturday, December 8, 2012</a></p>
95-
</blockquote>
89+
<blockquote cite="https://www.facebook.com/notes/facebook-engineering/under-the-hood-the-javascript-sdk-truly-asynchronous-loading/10151176218703920/" class="fb-xfbml-parse-ignore" fallback=""><!--blockquote_contents--></blockquote>
9690
</amp-facebook>
9791
' . PHP_EOL,
9892
],
@@ -101,10 +95,7 @@ public function get_raw_embed_dataset() {
10195
'https://www.facebook.com/photo.php?fbid=10102533316889441&set=a.529237706231.2034669.4&type=3&theater' . PHP_EOL,
10296
'
10397
<amp-facebook width="500" height="400" data-href="https://www.facebook.com/photo.php?fbid=10102533316889441&amp;set=a.529237706231.2034669.4&amp;type=3&amp;theater" data-embed-as="post" layout="responsive">
104-
<blockquote cite="https://www.facebook.com/photo.php?fbid=10102533316889441&amp;set=a.529237706231&amp;type=3" class="fb-xfbml-parse-ignore" fallback="">
105-
<p>Meanwhile, Beast turned to the dark side.</p>
106-
<p>Posted by <a href="https://www.facebook.com/zuck">Mark Zuckerberg</a> on <a href="https://www.facebook.com/photo.php?fbid=10102533316889441&amp;set=a.529237706231&amp;type=3">Friday, December 18, 2015</a></p>
107-
</blockquote>
98+
<blockquote cite="https://www.facebook.com/photo.php?fbid=10102533316889441&amp;set=a.529237706231&amp;type=3" class="fb-xfbml-parse-ignore" fallback=""><!--blockquote_contents--></blockquote>
10899
</amp-facebook>
109100
' . PHP_EOL,
110101
],
@@ -113,19 +104,7 @@ public function get_raw_embed_dataset() {
113104
'https://www.facebook.com/zuck/videos/10102509264909801/' . PHP_EOL,
114105
'
115106
<amp-facebook width="500" height="400" data-href="https://www.facebook.com/zuck/videos/10102509264909801/" data-embed-as="video" layout="responsive">
116-
<blockquote cite="https://www.facebook.com/zuck/videos/10102509264909801/" class="fb-xfbml-parse-ignore" fallback="">
117-
<p><a href="https://www.facebook.com/zuck/videos/10102509264909801/"></a></p>
118-
<p>I want to share a few more thoughts on the Chan Zuckerberg Initiative before I just start posting photos of me and Max for a while 🙂</p>
119-
<p>I hope one idea comes through: that we as a society should make investments now to ensure this world is much better for the next generation.</p>
120-
<p>I don\'t think we do enough of this right now. </p>
121-
<p>Sure, there are many areas where investment now will solve problems for today and also improve the world for the future. We do muster the will to solve some of those.</p>
122-
<p>But for the problems that will truly take decades of investment before we see any major return, we are dramatically underinvested.</p>
123-
<p>One example is basic science research to cure disease. Another is developing clean energy to protect the world for the future. Another is the slow and steady improvement to modernize schools. Others are systematic issues around poverty and justice. There is a long list of these opportunities.</p>
124-
<p>The role of philanthropy is to invest in important areas that companies and governments aren\'t funding — either because they may not be profitable for companies or because they are too long term for people to want to invest now.</p>
125-
<p>In the case of disease, basic research often needs to be funded before biotech or pharma companies can create drugs to help people. If we invest more in science, we can make faster progress towards curing disease.</p>
126-
<p>Our investment in the Chan Zuckerberg Initiative is small compared to what the world can invest in solving these great challenges. My hope is that our work inspires more people to invest in these longer term issues. If we can do that, then we can all really make a difference together.</p>
127-
<p>Posted by <a href="https://www.facebook.com/zuck">Mark Zuckerberg</a> on Friday, December 4, 2015</p>
128-
</blockquote>
107+
<blockquote cite="https://www.facebook.com/zuck/videos/10102509264909801/" class="fb-xfbml-parse-ignore" fallback=""><!--blockquote_contents--></blockquote>
129108
</amp-facebook>
130109
' . PHP_EOL,
131110
],
@@ -138,16 +117,12 @@ public function get_raw_embed_dataset() {
138117
'post_with_fallbacks' => [
139118
'
140119
<div class="fb-post" data-href="https://www.facebook.com/20531316728/posts/10154009990506729/" data-width="500" data-show-text="true">
141-
<blockquote cite="https://developers.facebook.com/20531316728/posts/10154009990506729/" class="fb-xfbml-parse-ignore">
142-
Posted by <a href="https://www.facebook.com/facebook/">Facebook</a> on <a href="https://developers.facebook.com/20531316728/posts/10154009990506729/">Thursday, August 27, 2015</a>
143-
</blockquote>
120+
<blockquote cite="https://developers.facebook.com/20531316728/posts/10154009990506729/" class="fb-xfbml-parse-ignore"><!--blockquote_contents--></blockquote>
144121
</div>
145122
',
146123
'
147124
<amp-facebook width="500" height="400" data-href="https://www.facebook.com/20531316728/posts/10154009990506729/" data-show-text="true" data-embed-as="post" layout="responsive">
148-
<blockquote cite="https://developers.facebook.com/20531316728/posts/10154009990506729/" class="fb-xfbml-parse-ignore" fallback="">
149-
<p> Posted by <a href="https://www.facebook.com/facebook/">Facebook</a> on <a href="https://developers.facebook.com/20531316728/posts/10154009990506729/">Thursday, August 27, 2015</a></p>
150-
</blockquote>
125+
<blockquote cite="https://developers.facebook.com/20531316728/posts/10154009990506729/" class="fb-xfbml-parse-ignore" fallback=""><!--blockquote_contents--></blockquote>
151126
</amp-facebook>
152127
',
153128
],
@@ -161,16 +136,14 @@ public function get_raw_embed_dataset() {
161136
'
162137
<div class="fb-page" data-href="https://www.facebook.com/xwp.co/" data-width="340" data-height="432" data-hide-cover="true" data-show-facepile="true" data-show-posts="false">
163138
<div class="fb-xfbml-parse-ignore">
164-
<blockquote cite="https://www.facebook.com/xwp.co/"><a href="https://www.facebook.com/xwp.co/">Like Us</a></blockquote>
139+
<blockquote cite="https://www.facebook.com/xwp.co/"><!--blockquote_contents--></blockquote>
165140
</div>
166141
</div>
167142
',
168143
'
169144
<amp-facebook-page width="340" height="432" data-href="https://www.facebook.com/xwp.co/" data-hide-cover="true" data-show-facepile="true" data-show-posts="false" layout="responsive">
170145
<div class="fb-xfbml-parse-ignore" fallback="">
171-
<blockquote cite="https://www.facebook.com/xwp.co/">
172-
<p><a href="https://www.facebook.com/xwp.co/">Like Us</a></p>
173-
</blockquote>
146+
<blockquote cite="https://www.facebook.com/xwp.co/"><!--blockquote_contents--></blockquote>
174147
</div>
175148
</amp-facebook-page>
176149
',
@@ -257,6 +230,9 @@ public function test__raw_embed_sanitizer( $source, $expected ) {
257230

258231
$content = AMP_DOM_Utils::get_content_from_dom( $dom );
259232

233+
// Normalize blockquote contents to account for editing of published posts or variability of localized datetime strings.
234+
$content = preg_replace( '#(<blockquote.*?>).+?(</blockquote>)#s', '$1<!--blockquote_contents-->$2', $content );
235+
260236
$this->assertEqualMarkup( $expected, $content );
261237
}
262238

‎tests/php/validation/test-class-amp-validation-manager.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -604,6 +604,7 @@ public function test_get_amp_validity_rest_field() {
604604
static function ( $error ) {
605605
return [
606606
'sanitized' => false,
607+
'title' => 'Unknown error (test)',
607608
'error' => $error,
608609
'status' => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_REJECTED_STATUS,
609610
'term_status' => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_REJECTED_STATUS,

0 commit comments

Comments
 (0)