#57787 closed defect (bug) (fixed)
WP_HTML_Tag_Processor::seek() off by 1 byte when seeking bookmarks set on a closing tag
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.2 | Priority: | normal |
Severity: | normal | Version: | 6.2 |
Component: | HTML API | Keywords: | gutenberg-merge has-patch has-unit-tests commit |
Focuses: | Cc: |
Description
The HTML API was introduced in #57575. The seek()
functionality is off by 1 position when seeking to find bookmarks set on a closing tag.
Setting a bookmark on a tag should set its "start" position before the opening "<", e.g.:
<div> Testing a <b>Bookmark</b> ----------------^
The current calculation assumes this is always one byte to the left from $tag_name_starts_at
.
However, in tag closers that index points to a solidus symbol "/":
<div> Testing a <b>Bookmark</b> ----------------------------^
The bookmark should therefore start two bytes before the tag name:
<div> Testing a <b>Bookmark</b> ---------------------------^
Change History (12)
This ticket was mentioned in PR #4115 on WordPress/wordpress-develop by @zieladam.
2 years ago
#1
- Keywords has-patch has-unit-tests added; needs-patch removed
#2
@
2 years ago
- Owner set to hellofromTonya
- Status changed from new to reviewing
Self-assigning for testing and commit review.
#4
@
2 years ago
- Keywords commit added
Patch: https://github.com/WordPress/wordpress-develop/pull/4115
- Before applying
WP_HTML_Tag_Processor
changes, the test fails 🔴 - After applying the
WP_HTML_Tag_Processor
changes, the test passes 🟢
The changes work as described ✅ Ready for commit 👍
#5
@
2 years ago
@zieladam I don't see an upstream issue or PR applying this same change in Gutenberg. Are you planning to backport these changes to Gutenberg to keep both in sync?
#6
@
2 years ago
With additional PR approvals from @flixos90 and @dmsnell, I'm prepping the commit now.
@hellofromTonya commented on PR #4115:
2 years ago
#7
Committed via https://core.trac.wordpress.org/changeset/55407.
#8
follow-up:
↓ 10
@
2 years ago
@hellofromTonya yes I am. We're still working out a specific workflow for backporting these patches – it could be a single PR per patch or a batch PR every two weeks. Let's keep this issue open for now.
#10
in reply to:
↑ 8
@
2 years ago
Replying to zieladam:
@hellofromTonya yes I am. We're still working out a specific workflow for backporting these patches – it could be a single PR per patch or a batch PR every two weeks. Let's keep this issue open for now.
Care must be given to if it's a bug fix for 6.2 and when the patch is introduced in the cycle. Beta 4 is next week and then a week later is RC1.
This particular ticket is targeted at one specific fix. Thinking [55407] resolves it. That said, if a follow-up PR is needed to fix something related to bookmark closing tag for 6.2, then please reopen the ticket; else, let's open a new Trac ticket.
#11
@
2 years ago
Good notes Tonya. Backport incoming: https://github.com/WordPress/gutenberg/pull/48378
## Description
With this PR
seek()
correctly finds bookmarks set on tag closers. It:array( 'tag_closers' => 'visit' )
inseek()
### About bookmark start indices:
Setting a bookmark on a tag should set its "start" position before the opening "<", e.g.:
The current calculation assumes this is always one byte to the left from $tag_name_starts_at.
However, in tag closers that index points to a solidus symbol "/":
The bookmark should therefore start two bytes before the tag name:
Trac ticket: https://core.trac.wordpress.org/ticket/57787
cc @ockham @dmsnell @gziolo