Opened 4 weeks ago
Last modified 2 days ago
#65253 reviewing defect (bug)
Superfluous whitespace in comment cookie consent markup
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 7.1 | Priority: | normal |
| Severity: | normal | Version: | 5.3 |
| Component: | Comments | Keywords: | has-patch needs-dev-note |
| Focuses: | Cc: |
Description
I always wondered why the checkboxes in my blog are not correctly aligned. Today I looked at it and found a superfluous space in the markup.
The code can be seen here:
https://github.com/WordPress/WordPress/blob/fe7b727fe416a39bef68df0aeaf2a143c5fd3763/wp-includes/comment-template.php#L2576-L2595
This is the relevant part:
<p class="comment-form-cookies-consent">%s %s</p>
The first placeholder is containing the input field and the second placeholder is the label.
In the source code of the page it looks like this:
<input id="wp-comment-cookies-consent" name="wp-comment-cookies-consent" type="checkbox" value="yes" /> <label for="wp-comment-cookies-consent">
As you can see, there is a space between the two HTML tags.
Typically this margin is better controlled by CSS, like this:
.comment-form #wp-comment-cookies-consent {
margin: 0 10px 0 0;
}
Every default theme has this margin right next to the input field:
https://github.com/search?q=repo%3AWordPress%2FWordPress+%23wp-comment-cookies-consent+language%3ACSS+%22margin%3A+0+10px+0+0%3B%22&type=code
The space is adding an extra margin. And there is no easy way to fix this, as this space is only seen in the source code.
I've checked the themes in the official directory and only 679 themes are using this CSS ID:
https://veloria.dev/search/2d4217d1-3ca1-4bca-8557-771f355df322
Some of those themes even don't add a margin to the right and some are just adding 5px, to match the 10px (if you add the space), so maybe this is intentional to have space between Label and text even if there is no CSS for this in the theme.
In the first iteration of this feature there was no space between this tags:
#43436
The space was introduced with this changeset:
https://core.trac.wordpress.org/changeset/46088
The suggested fix would be to remove the space:
<p class="comment-form-cookies-consent">%s%s</p>
But this would need a dev note for theme devs, who maybe worked around this or haven't added any margin at all.
Attachments (2)
Change History (10)
This ticket was mentioned in PR #12032 on WordPress/wordpress-develop by @masteradhoc.
9 days ago
#1
- Keywords has-patch added
#2
@
9 days ago
- Keywords needs-dev-note added
@zodiac1978 Thanks for the detailed write-up! I've opened a PR removing the space per your suggestion.
As default themes already seem to support this change i guess a dev-note would make sense!
#3
@
8 days ago
- Milestone changed from Awaiting Review to 7.1
- Owner set to masteradhoc
- Status changed from new to assigned
#4
@
6 days ago
- Owner changed from masteradhoc to desrosj
- Status changed from assigned to reviewing
I was discussing this with @zodiac1978 today at WCEU Contributor Day.
The change itself makes sense. I'd like to confirm that all of the default themes will not be adversely effected by this. If someone could test this, that would be great.
We also need to be careful because themes have updated their CSS accordingly, so I agree with @zodiac1978's tagging of this as needs-dev-note. He is going to look at drafting one so we can publish that once this change is made. The best recommendation would likely be some form of calling wp_add_inline_style() wrapped in an if ( version_compare() ) checking for <= 7.1 but >= 5.3 to output the styles required to properly space after the problematic change and before the fix.
#5
@
6 days ago
@peterwilsoncc came to me at WCEU contributor day and suggested an interesting different approach. We could use display: flex on .comment-form-cookies-consent. This would mean we don't need to change the markup anymore.
Unfortunately this comes with a slight change in styling if the text is over two lines.
I will upload two screenshots showing the effect.
#6
@
6 days ago
I've put together a jsbin to show the layout changes using flex-box rather than removing the space. See https://jsbin.com/hanuhid/edit?html,css,output
Removing the space may cause issues in third-party themes using negative margins to remove the space, whereas introducing flex-box for the WordPress default themes will ensure the layout is consistent.
#7
@
6 days ago
Twenty Twenty and Twenty Twenty-One already use display: flex. Twenty Twenty also has align-items: baseline, but that seems inappropriate with Twenty Twenty-One.
Removing the space may cause issues in third-party themes
Extra space is better than insufficient space. Also, if someone adds another checkbox to the form, it could have a space before the label, with or without similar margin or padding.
#8
@
2 days ago
True, valid approach! If the team would like to go this way, an alternative PR would be great.
I'd still keep the PR focused on removing the space, so new themes and themes that haven't addressed this yet have a proper, clean base to work from going forward.
But as said i see both options as valid approaches.
This removes the space between the checkbox input and its label in the comment cookie consent markup. The
comment-form-cookies-consentparagraph currently renders its two placeholders as%s %s, which inserts an extra whitespace character between the<input>and<label>tags in the output HTML.Because themes usually control the spacing between the checkbox and label via CSS this stray space stacks on top of the intended margin and misaligns the checkbox. The space was introduced in [46088] and was not present in the original implementation (#43436).
Trac ticket: https://core.trac.wordpress.org/ticket/65253
## Use of AI Tools
-