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.

Sign up for GitHub

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

Jump to bottom

Trim footnote anchors from excerpts #52518

Merged
merged 2 commits into from
Jul 12, 2023
Merged

Trim footnote anchors from excerpts #52518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
38b8cb9
Trim footnote anchors from excerpts
mcsf Jul 11, 2023
bb26cac
Add comments, fix spacing, appease linter
mcsf Jul 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 25 additions & 0 deletions lib/compat/wordpress-6.3/footnotes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php
/**
* Compatiblity filters for improved footnotes support.
*
* In core, this could be fixed directly in key functions.
*
* @package gutenberg
*/

/**
* Trims footnote anchors from content.
*
* @param string $content HTML content.
* @return string Filtered content.
*/
function gutenberg_trim_footnotes( $content ) {
if ( ! doing_filter( 'get_the_excerpt' ) ) {
return $content;
}

static $footnote_pattern = '_<sup data-fn="[^"]+" class="[^"]+">\s*<a href="[^"]+" id="[^"]+">\d+</a>\s*</sup>_';
Copy link
Member

Choose a reason for hiding this comment

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

This is working pretty well, thanks! I'm not sure what else we could do here.

Looks like it handles most of the inline changes we can make to the footnote sup

Screenshot 2023-07-12 at 1 33 43 pm

I only think with regexes like these, especially for new functionality, it would be great to have unit tests. That way when breaking changes come down the line we can react immediately.

Copy link
Member

Choose a reason for hiding this comment

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

Also tested in Chinese and Cyrillic scripts just to make sure no i18n funny business was messing with things.

Screenshot 2023-07-12 at 1 43 34 pm

Looks good!

Copy link
Member

Choose a reason for hiding this comment

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

Could we use the HTML tag processor here? Cc @dmsnell

Copy link
Member

@ramonjd ramonjd Jul 17, 2023 •
edited

Choose a reason for hiding this comment

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

Also interesting to know this.

When doing the backport I had a quick look at whether it could remove tags like it could with attributes/classes and I couldn't find anything.

The other option would be a DOM parser to strip the <sup /> and its contents but not sure if that would be a faster way to do it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any other possible tag inside of here besides <sup> and <a>?
The Tag Processor cannot easily remove this pattern, but the HTML Processor would be able to if I added <sup> support and the ability to remove a tag.

This could all plausibly happen for the WordPress 6.4 release, no promises.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also there's an "ugly" way to do it with the Tag Processor by subclassing and accessing internals, but the HTML Processor is working well enough I'm not experimenting with that approach anymore.

Copy link
Member

Choose a reason for hiding this comment

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

Could we use the HTML tag processor here?

I don't believe so because WP_HTML_Tag_Processor doesn't have a remove_tag method. If it did, you could do something like this:

$p = new WP_HTML_Tag_Processor( $html );
while ( $p->next_tag( array( 'tag_name' => 'SUP' ) ) ) {
	if (
		$p->get_attribute( 'data-fn' ) &&
		$p->get_attribute( 'class' )
	) {
		$p->set_bookmark( 'footer_sup' );
		if (
			$p->next_tag() &&
			'A' === $p->get_tag() &&
			$p->get_attribute( 'href' ) &&
			$p->get_attribute( 'id' )
		) {
			$p->seek( 'footer_sup' );
			$p->remove_tag(); // 👈 This method doesn't exist.
		}
		$p->release_bookmark( 'footer_sup' );
	}
}

But @dmsnell would know better 😄

Copy link
Member

Choose a reason for hiding this comment

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

(Oops race condition)

Copy link
Member

Choose a reason for hiding this comment

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

I'm just flagging this with @dmsnell as another place we're processing HTML 😄

return preg_replace( $footnote_pattern, '', $content );
}

add_filter( 'the_content', 'gutenberg_trim_footnotes' );
1 change: 1 addition & 0 deletions lib/load.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ function gutenberg_is_experiment_enabled( $name ) {
require_once __DIR__ . '/compat/wordpress-6.3/link-template.php';
require_once __DIR__ . '/compat/wordpress-6.3/block-patterns.php';
require_once __DIR__ . '/compat/wordpress-6.3/class-gutenberg-rest-blocks-controller.php';
require_once __DIR__ . '/compat/wordpress-6.3/footnotes.php';

// Experimental.
if ( ! class_exists( 'WP_Rest_Customizer_Nonces' ) ) {
Expand Down