Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#58007 closed defect (bug) (fixed)

HTML API: Support comments created by invalid tag name in tag closers

Reported by: dmsnell's profile dmsnell Owned by: bernhard-reiter's profile Bernhard Reiter
Milestone: 6.2.1 Priority: normal
Severity: normal Version: 6.2
Component: HTML API Keywords: has-patch has-unit-tests
Focuses: Cc:

Description (last modified by sabernhardt)

Adds support to the Tag Processor for a form of HTML comment derived from tag closers with invalid tag names. While it's the case for tag openers that an invalid tag name signals the parser to treat the syntax as normal text, if the token encountered is a tag closer then it starts an HTML comment until the first >

For example, <3 cannot start an HTML element because no tag name can start with a digit. However, </3 abides by a different rule. This is structural content and starts a comment. The comment should span until the first > in the text.

GitHub PR #4256

Side note: can we get a new "Component" in Trac for the "HTML API"?

Change History (21)

This ticket was mentioned in PR #4256 on WordPress/wordpress-develop by @dmsnell.


2 years ago
#1

  • Keywords has-patch has-unit-tests added

## Status

  • [x] Satisfy linter requirements
  • x Create Trac ticket #58007-trac
  • [ ] Expand comments, docblocks, add version and Trac ticket refereces
  • [ ] Create backport in Gutenberg

## What

Adds support for a few invalid HTML comment forms.

## Why

We want to continue to find and handle edge cases in HTML so that the Tag Processor remains reliable in the face of unexpected inputs.

## How

Captures the patterns described in the HTML specification for parse errors relating to invalid comment construction. Since these are comments it's appropriate to proceed consuming input until the end, and there's no need to look for other tags or attributes in the process.

cc: @adamziel @gziolo

This ticket was mentioned in PR #4256 on WordPress/wordpress-develop by @dmsnell.


2 years ago
#2

## Status

  • [x] Satisfy linter requirements
  • x Create Trac ticket #58007-trac
  • [ ] Expand comments, docblocks, add version and Trac ticket refereces
  • [ ] Create backport in Gutenberg

## What

Adds support for a few invalid HTML comment forms.

## Why

We want to continue to find and handle edge cases in HTML so that the Tag Processor remains reliable in the face of unexpected inputs.

## How

Captures the patterns described in the HTML specification for parse errors relating to invalid comment construction. Since these are comments it's appropriate to proceed consuming input until the end, and there's no need to look for other tags or attributes in the process.

cc: @adamziel @gziolo

#3 @sabernhardt
2 years ago

  • Component changed from General to Editor
  • Description modified (diff)

#4 @SergeyBiryukov
2 years ago

  • Component changed from Editor to HTML API

#5 follow-up: @dmsnell
2 years ago

Thank you @SergeyBiryukov for creating that category, if you did that!

#6 in reply to: ↑ 5 @SergeyBiryukov
2 years ago

@dmsnell Thank you for suggesting it :)

#7 @costdev
2 years ago

  • Milestone changed from Awaiting Review to 6.2.1

Moving to 6.2.1 for visibility.

Last edited 2 years ago by costdev (previous) (diff)

#8 @dmsnell
2 years ago

Does anyone have any further thoughts on this? If not, would it be fine to merge in?

#9 @costdev
2 years ago

@dmsnell I just left a few docblock-related suggestions on the PR 🙂

@dmsnell commented on PR #4256:


2 years ago
#10

@costdev I think I've addressed all your comments now, thanks. must say I tried to get the docblock ordering and formatting right, but for some reason that's consistently hard to do

#11 @dmsnell
2 years ago

Thank you @costdev - I believe I have addressed your comments

@dmsnell commented on PR #4256:


2 years ago
#12

So there were @costdev - sorry for missing that. I'll double-check once the tests finish again to make sure no remain.

#13 @dmsnell
2 years ago

Looks like this has been sitting without any further comment or issue for around a week. Is it okay to merge or are there any outstanding concerns?

#14 @dmsnell
2 years ago

@ockham I've updated the description in the GitHub PR, added comments about the extra syntax forms for which this patch adds support. a note here: it's not possible to visit these new forms, because they do not represent tags, but it's important that the processor doesn't get off-track when it encounters them.

#15 @Bernhard Reiter
2 years ago

  • Owner set to Bernhard Reiter
  • Resolution set to fixed
  • Status changed from new to closed

In 55667:

HTML API: Add support for a few invalid HTML comment forms.

  • Comments created by means of a tag closer with an invalid tag name, e.g. </3>.
  • Comments closed with the invalid --!> closer. (Comments should be closed by --> but if the ! appears it will also close it, in error.)
  • Empty tag name elements, which are technically skipped over and aren't comments, e.g. </>.

Props dmsnell, costdev.
Fixes #58007.

#16 @dmsnell
2 years ago

Thanks @bernhard-reiter!

@Bernhard Reiter commented on PR #4256:


2 years ago
#17

Committed to Core trunk in `r55667`.

#18 @Bernhard Reiter
2 years ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for 6.2.1 consideration.

#19 @Bernhard Reiter
2 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 55668:

HTML API: Add support for a few invalid HTML comment forms.

  • Comments created by means of a tag closer with an invalid tag name, e.g. </3>.
  • Comments closed with the invalid --!> closer. (Comments should be closed by --> but if the ! appears it will also close it, in error.)
  • Empty tag name elements, which are technically skipped over and aren't comments, e.g. </>.

Props dmsnell, costdev.
Merges [55667] to the 6.2 branch.
Fixes #58007.

@Bernhard Reiter commented on PR #4256:


2 years ago
#20

Backported to Core's 6.2 branch in `r55668`.

#21 @Bernhard Reiter
2 years ago

  • Keywords fixed-major removed
Note: See TracTickets for help on using tickets.