Skip to content

Remove escaping of title in legacy post templates#5026

Merged
westonruter merged 1 commit intodevelopfrom
fix/emoji-in-post-title
Jul 11, 2020
Merged

Remove escaping of title in legacy post templates#5026
westonruter merged 1 commit intodevelopfrom
fix/emoji-in-post-title

Conversation

@westonruter
Copy link
Copy Markdown
Member

Summary

Given a post with an emoji in the title:

image

The title was getting erroneously escaped in the h1 of the legacy post templates:

image

This PR removes the escaping to prevent that from happening:

image

Note that escaping is not required necessary for two reasons:

  • Core doesn't escape post titles in the_title() because it can include markup; the value returned by $this->get( 'post_title' ) comes from get_the_title( $this->post ).
  • The AMP plugin's sanitizers will prevent bad markup from being output.

This issue was introduced a long time ago in 6b4de15 with this change:

-		<h1 class="amp-wp-title"><?php echo wp_kses_data( $this->get( 'post_title' ) ); ?></h1>
+		<h1 class="amp-wp-title"><?php echo esc_html( $this->get( 'post_title' ) ); ?></h1>

Originally reported by @delans on Twitter.

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@westonruter westonruter added Bug Something isn't working Reader Mode labels Jul 11, 2020
@westonruter westonruter added this to the v1.6 milestone Jul 11, 2020
@westonruter westonruter requested a review from pierlon July 11, 2020 17:48
@googlebot googlebot added the cla: yes Signed the Google CLA label Jul 11, 2020
@github-actions
Copy link
Copy Markdown
Contributor

Plugin builds for cd9085a are ready 🛎️!

Copy link
Copy Markdown
Member

@amedina amedina left a comment

Choose a reason for hiding this comment

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

Ship it.

@westonruter westonruter merged commit f891c4d into develop Jul 11, 2020
@westonruter westonruter deleted the fix/emoji-in-post-title branch July 11, 2020 22:15
westonruter added a commit that referenced this pull request Jul 12, 2020
…eme-support-paired-default

* 'develop' of github.com:ampproject/amp-wp:
  Update dependency dealerdirect/phpcodesniffer-composer-installer to v0.7.0
  Harden ReaderThemesTest when custom theme directories present
  Update dependency sirbrillig/phpcs-variable-analysis to v2.8.3
  Skip test in WP 4.9
  Fix grammar typo
  Remove escaping of title in legacy post templates (#5026)
  Skip test when Twenty Twenty is not installed
  Prevent Cover Template section in AMP Customizer from appearing when Twenty Twenty used as Reader theme
  Add is_theme_overridden method on ReaderThemeLoader service
  Improve detection for whether or not ReaderThemeSwitcher is enabled
  Make ReaderThemeLoaer unconditional and add method to obtain originally-active theme
  Prevent sanitization of common style elements in the Customizer preview
  Fix lint issue in test
  Fix lint issue
  Add test
  Prevent sanitization of styles in some Core themes requried for some Customizer features to work correctly
@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Jul 18, 2020
@westonruter westonruter added the WS:Core Work stream for Plugin core label Aug 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working Changelogged Whether the issue/PR has been added to release notes. cla: yes Signed the Google CLA Reader Mode WS:Core Work stream for Plugin core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants