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

get_root_layout_rules: remove unnecessary call to sanitize_title #53568

Merged
merged 1 commit into from
Aug 14, 2023

Conversation

oandregal
Copy link
Member

@oandregal oandregal commented Aug 11, 2023

Part of #45171
Extracted from #53351

What

This removes an unnecessary call to sanitize_title.

Why

When this was introduced at #40875 layout definitions were taken from theme.json (user modifiable), so sanitization was required. Now, the layout definitions are static, so we no longer need this.

I took a profile from loading the "hello world" post in a site that uses TwentyTwentyThree. I don't expect this to have big performance wins, though it's a small and fast win.

Trunk PR
Called 92 times Called 84 times
Captura de ecrã 2023-08-11, às 14 14 48 Captura de ecrã 2023-08-11, às 14 14 54

Testing Instructions

Verify tests pass.

The layout definitions are static and defined by the code,
so there is no need to sanitize them.
@github-actions
Copy link

This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress.

If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged.

If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack.

Thank you! ❤️

View changed files
❔ lib/class-wp-theme-json-gutenberg.php

@oandregal oandregal self-assigned this Aug 11, 2023
@oandregal oandregal added the [Type] Performance Related to performance efforts label Aug 11, 2023
Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

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

Nice follow-up 👍

✅ Removing sanitize_title here sounds safe to me, as we can always expect gutenberg_get_layout_definitions to return a safe classname, as you mention
✅ Smoke tested on my test site that block-level and root layout classnames continue to be output as expected

LGTM! ✨

@andrewserong andrewserong added the [Feature] Layout Layout block support, its UI controls, and style output. label Aug 14, 2023
@oandregal oandregal merged commit edc9cbc into trunk Aug 14, 2023
52 of 53 checks passed
@oandregal oandregal deleted the remove/unnecessary-sanitize-title-in-layout branch August 14, 2023 07:00
@github-actions github-actions bot added this to the Gutenberg 16.5 milestone Aug 14, 2023
@mikachan mikachan added the Needs PHP backport Needs PHP backport to Core label Sep 5, 2023
@SiobhyB SiobhyB removed the Needs PHP backport Needs PHP backport to Core label Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Layout Layout block support, its UI controls, and style output. [Type] Performance Related to performance efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants