Opened 5 months ago
Last modified 3 months ago
#65253 reviewing defect (bug)
Superfluous whitespace in comment cookie consent markup
| Reported by: | zodiac1978 | Owned by: | desrosj |
|---|---|---|---|
| Priority: | normal | Milestone: | Future Release |
| Component: | Comments | Version: | 5.3 |
| Severity: | normal | Keywords: | has-patch needs-dev-note |
| Cc: | Focuses: |
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 (3)
Change History (17)
This ticket was mentioned in PR #12032 on WordPress/wordpress-develop by @masteradhoc.
5 months ago
#1
- Keywords has-patch added
#2
@
5 months 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!
#4
follow-up:
↓ 11
@
4 months ago
- Owner changed from to
- Status assigned → 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
@
4 months 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
@
4 months 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
@
4 months 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
@
4 months 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 ticket was mentioned in Slack in #core by amykamala. View the logs.
4 months ago
@masteradhoc commented on PR #12032:
3 months ago
#10
Close to allow the work to be continued on the alternative approach.
#11
in reply to: ↑ 4
@
3 months ago
Replying to desrosj:
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.
I've checked every default theme and found three groups:
1: Is adding a margin of 10 pixels to the checkbox. Removing the space fixes too much space.
2010-2019
2: Is using flex on this element and the space does not change the look (is ignored)
2020, 2021 and 2023-2025
3: Is NOT using flex and NOT adding a margin to the checkbox. Removing the space leads to a broken layout, as the text is directly next to the checkbox.
2022
So we need to adjust "Twenty Twenty-Two" (either switching to flex or adding the margin).
@
3 months ago
Twenty Twenty-Two with the patch added (space removed) and now with too less space between the items
#12
@
3 months ago
Flex patch (baseline if we want to center the checkbox vertically):
.wp-block-post-comments .comment-form-cookies-consent {
align-items: baseline;
display: flex;
gap: 0.25em;
}
Margin patch (as in 2010-2019):
.comment-form #wp-comment-cookies-consent {
margin: 0 10px 0 0;
}
#13
follow-up:
↓ 14
@
3 months ago
- Milestone 7.1 → Future Release
It is very late in the cycle to make any change that requires a dev note, though I do not agree that theme authors should need to add spacing if it was unnecessary for them since WordPress 5.3.
If the space is inappropriate in specific themes, they could add a filter:
add_filter( 'comment_form_field_cookies', function( $field ) {
$field = str_replace(
'> <label ',
'><label ',
$field
);
return $field;
});
why the checkboxes in my blog are not correctly aligned
If the form has multiple checkboxes, any style targeting either #wp-comment-cookies-consent or its container likely would cause an alignment discrepancy between the checkboxes' labels. Removing the space character would not fix it.
need to adjust "Twenty Twenty-Two"
Twenty Twenty-Two uses the legacy Comments block, which has a problem with the markup plus at least two reported CSS bugs. The CSS intentionally remains broken to encourage updating the block.
introducing flex-box for the WordPress default themes will ensure the layout is consistent
Replacing the 10-pixel margin with flex could be an improvement in themes from Twenty Ten to Twenty Nineteen.
.comment-form-cookies-consent {
align-items: baseline;
display: flex;
gap: 10px; /* Unnecessary in T16 and T17 */
}
#14
in reply to: ↑ 13
@
3 months ago
Replying to sabernhardt:
It is very late in the cycle to make any change that requires a dev note, though I do not agree that theme authors should need to add spacing if it was unnecessary for them since WordPress 5.3.
This is not the main reason for the dev note. And this summary does not really fit the reality. First: This is a regression. The space wasn't there before. This is a bug fix. Second: Many themes haven't styled this checkbox. The dev note is there to raise awareness, that this maybe needs to be added in your (classic) theme.
If the space is inappropriate in specific themes, they could add a filter:
As I said, this is a regression and should be fixed. The mentioned workaround has one big problem. Theme authors need to know what exactly happens here and most of them just look at the design and lower the margin instead of removing the space.
If we remove the space the worst what happens is that some themes now have maybe 5px instead of 10 px margin. And this is easily fixed and the dev note raises awareness to do that.
why the checkboxes in my blog are not correctly aligned
If the form has multiple checkboxes, any style targeting either
#wp-comment-cookies-consentor its container likely would cause an alignment discrepancy between the checkboxes' labels. Removing the space character would not fix it.
Of course it does. If a theme or plugins adds more checkboxes in this place (like in my blog), the margin gets applied to all checkboxes via style.css from the theme, but only the consent checkbox has the extra space. Removing it, aligns it with the other checkboxes. This is the whole reason of this ticket and why I am reporting this.
need to adjust "Twenty Twenty-Two"
Twenty Twenty-Two uses the legacy Comments block, which has a problem with the markup plus at least two reported CSS bugs. The CSS intentionally remains broken to encourage updating the block.
And per https://core.trac.wordpress.org/ticket/56496#comment:30 the CSS fix was still applied because the old block is still the default one used in this theme. And so we could just add one more CSS block to fix this too.
introducing flex-box for the WordPress default themes will ensure the layout is consistent
Replacing the 10-pixel margin with flex could be an improvement in themes from Twenty Ten to Twenty Nineteen.
If this is the decision of the component maintainer/core committer, I'm fine with it.
But one question: If we all agree that this is a bug/regression and should be fixed, but it misses the 7.1 cycle, why punt to future release and not 7.2? Or 7.0.3?
I don't know if there are rules for it, but it is very demotivating if a ticket which already had a milestone gets punted to maybe somewhere in the future ...
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
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
-