HTML API: Add docs and tests about lowercasing in class_list #6920
Conversation
The class_list method yields class name strings in lower case. This is intended but undocumented. Document and add tests for this behavior.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
|
@sirreal changes LGTM. |
|
@dmsnell Will you review these documentation changes? These would be good to backport to 6.6. |
Revert other documentation tweaks and only include changes to reflect the ASCII-lowercase behavior.
|
This may not be the best approach. Rather than documenting the specifics of the behavior here, it seems best to audit the class-related methods. |
| @@ -1694,13 +1694,13 @@ public function has_class( $wanted_class ) { | |||
| } | |||
|
|
|||
| /** | |||
| * Generator for a foreach loop to step through each class name for the matched tag. | |||
| * Generator that yields each class name for the matched tag. | |||
There was a problem hiding this comment.
Hm. the wording here was carefully chosen to clue people in on how this function is intended to be used. While I understand that yield is technically what's occurring on the low level of PHP details, but do you find that this change better leads people to understand how to use the method?
| * | ||
| * This generator function is designed to be used inside a "foreach" loop. | ||
| * Class names will be lowercased and unique class names will be yielded once. |
There was a problem hiding this comment.
I'm really uncomfortable both taking out relevant information for consumers of this method and then adding new kinds of specifications for things that aren't specified.
In particular I've attempted to avoid using the term lower-case and upper-case except where necessary. if you can find some notion in the CSS specification indicating that this is part of what it means to be a CSS class, then I think it's appropriate. I'm far less versed in the CSS spec so I've been reluctant to add constraints that aren't there.
To this end I've favored creating our own term to avoid the ambiguity, which is $comparable_name.
The best I can find is in CSS2 (repeated up through CSS Selectors, Level 4), which states
All CSS syntax is case-insensitive within the ASCII range…
and
All Selector syntax is ASCII case-insensitive…
The most confusing bit, which is what I was referring to earlier when we discussed this, is a note about quirks mode. In 6.6 Class selectors it says:
When matching against a document which is in quirks mode, class names must be matched ASCII case-insensitively; class selectors are otherwise case-sensitive, only matching class names they are identical to.
We obviously can't know about quirks mode in the Tag Processor, and I don't even think we want to, but I think this section is still speaking specifically about the class attribute and not the CSS classes.
Funny thing is, I can't find any mention of what a class is. All I can find in the CSS specifications are discussions about class selectors.
The DOM specification indicates that the classList getter is a token set whose token list object as known as "the element's classes."
It further hearkens back to the CSS spec when saying,
The comparisons for the classes must be done in an ASCII case-insensitive manner if root's node document's mode is "quirks"; otherwise in an identical to manner.
I don't think there's a specification on classList forcing it to be lower-case.
Safari/WebKit seems to do it by happenstance
Hang on, we're continuing this chat in person and you can ignore all this. it's partial and possibly out-dated.
There was a problem hiding this comment.
| No-Quirks Mode | Quirks Mode |
|---|---|
 |
 |
The class_list method yields class name strings in lower case. This is
intended but undocumented. Document and add tests for this behavior.
Trac ticket: https://core.trac.wordpress.org/ticket/61520
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.