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

Tag Processor: Add Bookmark Invalidation Logic #4116

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Feb 22, 2023

Targeting WP 6.3.

While WP_HTML_Tag_Processor currently only supports changing a given tag's attributes, we plan to also provide methods to make broader changes (possibly through a subclass of WP_HTML_Tag_Processor).
An API like that will have the potential of replacing a tag that a bookmark points to. As a preparation, we thus need to make sure that all bookmarks affected by a HTML replacement are invalidated (i.e. released).

This is done by extending the existing loop in apply_attributes_updates that adjusts bookmarks' start and end positions upon HTML changes to check if the entire bookmark is within a portion of the HTML that has been replaced.

Note that this PR is a backport of WordPress/gutenberg#47559.

Trac ticket: https://core.trac.wordpress.org/ticket/57788


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@ockham ockham self-assigned this Feb 22, 2023
@ockham ockham force-pushed the add/tag-processor-bookmark-invalidation-logic branch from 3217dba to 8f43d6e Compare February 22, 2023 16:52
@ockham ockham marked this pull request as ready for review February 22, 2023 16:53
@ockham ockham requested review from gziolo and adamziel February 22, 2023 16:53
@ockham
Copy link
Contributor Author

ockham commented Feb 22, 2023

cc/ @dmsnell 😊

Copy link
Member

@dmsnell dmsnell left a comment

Choose a reason for hiding this comment

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

Adding approval here, given that we already ran review in the Gutenberg repo.

I've also merged this into dmsnell/WordPress-develop html-api/wordpress-6.3

@ockham
Copy link
Contributor Author

ockham commented Feb 22, 2023

Thank you! 😊

Just noting that we can't merge this to trunk until Core is comes out of the 6.2 Beta phase, which should happen on March 7.

Copy link
Contributor

@hellofromtonya hellofromtonya left a comment

Choose a reason for hiding this comment

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

  • Confirmed this is a copy of Gutenberg PR 47559
  • Includes ticket annotation in tests ✅
  • Includes @since annotation ✅

Ready for commit 👍

@hellofromtonya
Copy link
Contributor

Prepping commit now ✅

@hellofromtonya
Copy link
Contributor

Committed via https://core.trac.wordpress.org/changeset/55555.

@ockham
Copy link
Contributor Author

ockham commented Mar 30, 2023

Thank you very much for landing this @hellofromtonya !

@ockham ockham deleted the add/tag-processor-bookmark-invalidation-logic branch March 30, 2023 13:16
dmsnell added a commit to WordPress/gutenberg that referenced this pull request Apr 20, 2023
 - Support a few extra invalid comment syntaxes. WordPress/wordpress-develop#4256
 - Invalidate bookmarks which have been eliminated by enqueued changes. WordPress/wordpress-develop#4116
 - Expose whether the currently-matched tag has the self-closing flag. WordPress/wordpress-develop#4266
 - Avoid double-writing an attribute value if given case-variations of the name. WordPress/wordpress-develop#4337
dmsnell added a commit to WordPress/gutenberg that referenced this pull request Apr 21, 2023
 - Support a few extra invalid comment syntaxes. WordPress/wordpress-develop#4256
 - Invalidate bookmarks which have been eliminated by enqueued changes. WordPress/wordpress-develop#4116
 - Expose whether the currently-matched tag has the self-closing flag. WordPress/wordpress-develop#4266
 - Avoid double-writing an attribute value if given case-variations of the name. WordPress/wordpress-develop#4337
 - Linting updates from WordPress/wordpress-develop#4360.
dmsnell added a commit to WordPress/gutenberg that referenced this pull request Apr 24, 2023
 - Support a few extra invalid comment syntaxes. WordPress/wordpress-develop#4256
 - Invalidate bookmarks which have been eliminated by enqueued changes. WordPress/wordpress-develop#4116
 - Expose whether the currently-matched tag has the self-closing flag. WordPress/wordpress-develop#4266
 - Avoid double-writing an attribute value if given case-variations of the name. WordPress/wordpress-develop#4337
 - Linting updates from WordPress/wordpress-develop#4360.
 - Avoid losing previous updates in certain cases when seeking earlier in a document. WordPress/wordpress-develop#4345
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.

3 participants