Html api/tag processor self closing flag#4266
Html api/tag processor self closing flag#4266dmsnell wants to merge 1 commit intoWordPress:trunkfrom
Conversation
| } | ||
|
|
||
| /** | ||
| * Indicates if the currently matched tag contains the self-closing flag. |
There was a problem hiding this comment.
Maybe
| * Indicates if the currently matched tag contains the self-closing flag. | |
| * Indicates if the currently matched tag contains the <self-closing/> flag. |
to make it a bit more illustrative? 🤔
There was a problem hiding this comment.
was trying to follow the explicit constraint not to use HTML tags in summaries and tags
No HTML markup or Markdown of any kind should be used in the summary. If the text refers to an HTML element or tag, then it should be written as “image tag” or “img” element, not “
“. - PHPDoc standards
| /** | ||
| * Data provider. HTML tags which might have a self-closing flag, and an indicator if they do. | ||
| * | ||
| * @return array[] |
There was a problem hiding this comment.
Nit:
| * @return array[] | |
| * @return array[]. |
There was a problem hiding this comment.
is this even correct? most in the file don't have that period, and I would expect a period on a description, not following a type.
in either case, it's clear we need to clean up the consistency of it in the file. mind if I file a follow-up PR to do that all at one time?
| return array( | ||
| // These should not have a self-closer, and will leave an element un-closed if it's assumed they are self-closing. | ||
| 'Self-closing flag on non-void HTML element' => array( '<div />', true ), | ||
| 'No self-closing flag on non-void HTML element' => array( '<div>', false ), |
There was a problem hiding this comment.
It's interesting the linter doesn't complain about a misaligned arrow 🤔 I hate to be that person, but all the arrows need to be at the same offset.
There was a problem hiding this comment.
then don't be that person 😄
I'll try this out, but I think the linter complains here if we line up the arrows. it has some line-length threshold at which it separates the alignment of shorter lines and longer lines.
There was a problem hiding this comment.
confirmed: the alignment that looks right is rejected because it doesn't follow the specific linting rules. now go away and take your legibility with you! 🙃
| 'Self-closing flag after extra spaces' => array( '<div />', true ), | ||
| 'Self-closing flag after attribute' => array( '<div id=test/>', true ), | ||
| 'Self-closing flag after quoted attribute' => array( '<div id="test"/>', true ), | ||
| 'Self-closing flag after boolean attribute' => array( '<div enabled/>', true ), |
There was a problem hiding this comment.
Would you mind adding two more tests? I'm thinking of:
<img/><- without a space between the tag name and a closer<img / ><-/is actually an HTML attribute because of the trailing space.
There was a problem hiding this comment.
gladly! these are great test cases to include, and thankfully it handles it right already.
|
LGTM! I just left a few minor notes. |
dmsnell
force-pushed
the
html-api/tag-processor-self-closing-flag
branch
2 times, most recently
from
c605371 to
5d032d3
Compare
Trac ticket: [#58009](https://core.trac.wordpress.org/ticket/58009#ticket) In this patch we're adding `has_self_closing_flag()` to the HTML Tag Processor. This exposes whether a currently-matched tag contains the self-closing flag `/`. This information is critical for the evolution of the HTML API in order to track and parse HTML structure, specifically, knowing whether an HTML foreign element is self-closing or not.
dmsnell
force-pushed
the
html-api/tag-processor-self-closing-flag
branch
2 times, most recently
from
6625190 to
18c1cb9
Compare
|
Committed to Core |
- 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
- 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.
- 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
3 participants
Introduce has_self_closing_flag() to the HTML Tag Processor, exposing whether a currently-matched tag contains the self-closing flag /, used when determining if one needs to look for a closing tag of the same name.
This enables structural parsing of an HTML document and is needed to identify self-closing HTML foreign elements.
Trac ticket #58009
cc: @adamziel @gziolo @ockham