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

Reuse save function from core Code block which includes escaping #118

Merged
merged 3 commits into from
Jun 21, 2020

Conversation

westonruter
Copy link
Owner

@westonruter westonruter commented Jun 21, 2020

When extending the Code block, there's no need for us to define our own save function since we can reuse the one from core entirely. The version we had lacked the required escape() function call, but by allowing the core version to pass through with the escape() function intact. This fixes block validation errors when the code content contains special HTML characters.

Fixes #110.

Build for testing: syntax-highlighting-code-block.zip

@westonruter
Copy link
Owner Author

@mkaz FYI: As this appears to also be an issue in your version.

@westonruter westonruter changed the title Add missing escape() of Code block content in save function Reuse save function from Code block which includes escaping Jun 21, 2020
Comment on lines -272 to -279
save({ attributes }) {
return (
<pre>
<code>{attributes.content}</code>
</pre>
);
},

Copy link
Owner Author

Choose a reason for hiding this comment

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

By deleting this here, we can just reuse what the overridden block is already doing, but with escape():

import { escape } from './utils';

export default function save( { attributes } ) {
	return (
		<pre>
			<code>{ escape( attributes.content ) }</code>
		</pre>
	);
}

Copy link
Owner Author

Choose a reason for hiding this comment

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

Actually, removing this is causing problems. In WordPress 5.4.1 without Gutenberg active, I'm getting an error where the save function in the blockType is undefined:

image

Copy link
Owner Author

Choose a reason for hiding this comment

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

For some reason settings.save is defined up front, after being filtered it gets dropped, even though all of settings is being returned:

return {
...settings,

Perhaps it is due to this apparent race condition I commented about on the subsequent lines:

/*
* @todo Why do the attributes need to be augmented here when they have already been declared for the block type in PHP?
* There seems to be a race condition, as wp.blocks.getBlockType('core/code') returns the PHP-augmented data after the
* page loads, but at the moment this filter calls it is still undefined.
*/

In any case, capturing the original save the first time we see it seems to fix this:

--- a/src/index.js
+++ b/src/index.js
@@ -26,6 +26,8 @@ import * as BlockEditor from '@wordpress/block-editor';
  */
 import languagesNames from './language-names';
 
+let save;
+
 /**
  * Extend code block with syntax highlighting.
  *
@@ -37,6 +39,14 @@ const extendCodeBlockWithSyntaxHighlighting = (settings) => {
 		return settings;
 	}
 
+	if (settings.save) {
+		save = settings.save;
+	}
+
+	if (!save) {
+		return settings;
+	}
+
 	const useLightBlockWrapper =
 		settings.supports &&
 		settings.supports.lightBlockWrapper &&
@@ -269,6 +279,8 @@ const extendCodeBlockWithSyntaxHighlighting = (settings) => {
 			);
 		},
 
+		save,
+
 		deprecated: [
 			...(settings.deprecated || []),
 			{

But this hardly seems like the right solution.

@youknowriad Do you know what's going on here?

Choose a reason for hiding this comment

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

I think that filter might run for deprecations too. So it's possible that one of them don't have save.

@westonruter westonruter changed the title Reuse save function from Code block which includes escaping Reuse save function from core Code block which includes escaping Jun 21, 2020
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

The plugin has been updated and it is working smoothly now. Thank you @westonruter for your unique help! I really appreciate that. :)

@westonruter westonruter merged commit fc10521 into develop Jun 21, 2020
@westonruter westonruter deleted the fix/save-function-escaping branch June 21, 2020 15:24
@mkaz
Copy link

mkaz commented Jun 23, 2020

@mkaz FYI: As this appears to also be an issue in your version.

Thanks @westonruter for the heads up.

Unfortunately I do need my own save function, since the Prism JS needs the additional attributes. So I'll need to add the escape() function directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Htaccess Syntax Errors
3 participants