Comments should be turned off on pages by default

[melchoyce]

In the vast majority of use cases, you do not want comments turned on for pages. We should turn them off by default and save site builders everywhere some agony.

[SergeyBiryukov]

Related: #12991, #17478, #27111.

[paaljoachim]

Hear hear!
Good call..:)

[tyxla]

Great idea IMHO. I'm not sure why this hasn't been brought so far.

I'm just wondering about the pingbacks/trackbacks (ping_status) - i believe this one should be disabled by default for pages, too? I'm not 100% sure though (second opinion would be needed here).

[tyxla]

Making comment_status = closed for pages by default. Adding a unit test for that.

[tyxla]

Making comment_status = closed and ping_status = closed for pages by default. Adding a unit test for those.

[tyxla]

I've just added 2 patches. The first one (31168-a.patch) makes the default comment_status for pages to be closed. The second one (31168-b.patch) also makes the ping_status for pages closed by default. Both patches include unit tests for testing the default values.

[tyxla]

Updating 31168-b.patch. This will also set ping_status to closed for the Sample Page that WordPress creates upon installation.

[utkarshd_42]

I changed the default value of Discussion metabox. Which is checked (by default in a new post or page). Dunno whether the best way. But works for me :)

[melchoyce]

Replying to tyxla:

Great idea IMHO. I'm not sure why this hasn't been brought so far.

I'm just wondering about the pingbacks/trackbacks (ping_status) - i believe this one should be disabled by default for pages, too? I'm not 100% sure though (second opinion would be needed here).

For sure, let's disable pingbacks/trackbacks as well by default on pages.

[tyxla]

Replying to melchoyce:

Replying to tyxla:

Great idea IMHO. I'm not sure why this hasn't been brought so far.

I'm just wondering about the pingbacks/trackbacks (ping_status) - i believe this one should be disabled by default for pages, too? I'm not 100% sure though (second opinion would be needed here).

For sure, let's disable pingbacks/trackbacks as well by default on pages.

Right, please refer to my 31168-b.2.patch for a solution that includes disabling ping_status for pages.

[wonderboymusic]

I would like to shepherd this

[rachelbaker]

Would a better approach here be to use post_type_supports()? Remove comments from the supports array when the page type is registered.

Sites could re-enable comments with add_post_type_support( 'page', 'comments' );

Using post_type_supports checks is how the REST API will determine what fields to include in endpoint responses.

[wonderboymusic]

yeah - I working through some helper functions that check post_type_supports()

[melchoyce]

How's this going? :)

[helen]

Related in terms of post_type_supports(): #28080.

[DrewAPicture]

I'd recommend that we probably punt this to a future release. @wonderboymusic still owns it, so that would be his call.

[mrahmadawais]

This also disables ping by default as per the patches of tyxla

[mrahmadawais]

Ignore the last patch, this one unchecks both the comments and pings inside the metabox when adding a new page

[DrewAPicture]

Definitely too late to consider this for 4.2. Pushing to a future release.

[jorbin]

@Wonderboymusic - since you owned this in 4.2, care to deliver us to the promised land in 4.3?

[obenland]

I really want to see this happen, but with beta only two weeks away, it might be too late for 4.3 as well.
What is left to do here @valensdesign?

[valendesigns]

Sorry, Menu Customizer stole all my time. I'll take a look at this over the weekend.

[obenland]

@valensdesign, any news here?

[fabioneves]

Replying to rachelbaker:

Would a better approach here be to use post_type_supports()? Remove comments from the supports array when the page type is registered.

Sites could re-enable comments with add_post_type_support( 'page', 'comments' );

Using post_type_supports checks is how the REST API will determine what fields to include in endpoint responses.

This solution from @rachelbaker is much cleaner imo.

[valendesigns]

@fabioneves I'm fairly sure we don't want to remove support for comments completely, just turn them off by default for new pages. Removing support drastically changes pre established behavior and would require a developer to turn them back on. I'm going to spend some time on this today and see if I can come up with a solution based on all the comments and Slack discussions.

[valendesigns]

The latest patch adds two new functions and filters that we can use to get & set the default comment and ping status on a per post_type basis. It also includes four unit tests, two of which would fail before the patch is applied as page & cpt do not support a closed default value when left empty in wp_insert_post or when the post_type does not support comments or trackbacks.

With this patch we can explicitly set the defaults on custom post types with a filter and still allow support for comments and trackbacks on those post types without having them set to open by default. Something I've personally been wanting for a while.

Whomever is still interested in seeing this move forward, please test and let me know your thoughts ASAP.

[efegue]

Replying to valendesigns:

The latest patch adds two new functions and filters that we can use to get & set the default comment and ping status on a per post_type basis. It also includes four unit tests, two of which would fail before the patch is applied as page & cpt do not support a closed default value when left empty in wp_insert_post or when the post_type does not support comments or trackbacks.

With this patch we can explicitly set the defaults on custom post types with a filter and still allow support for comments and trackbacks on those post types without having them set to open by default. Something I've personally been wanting for a while.

Whomever is still interested in seeing this move forward, please test and let me know your thoughts ASAP.

I'm going to test this later today, but how is this different from disabling comments from the page? You'll need a developer to turn it back anyway or am I wrong?

[valendesigns]

@efegue the previous patch was a global deactivation of comment & trackback support for the page post type. What this patch does is set comment_status & ping_status to closed for new pages and post types that do not support those features. From a UX standpoint you can still check the option to display comments and trackbacks on a new page, so you will not need a developer to get comments back.

However, this patch does allow you to filter the default status for other post types. So a developer would be able to support comments and trackbacks in their custom post type, but have those features default to closed when a new post is created.

From my point of view this change is a lot less destructive, or potentially confusing, to the user. As well, it give us more control as developers to make decisions about our custom post types and how they support comments and trackbacks. And finally, we could standardize the default values with these new functions in other parts of Core.

[efegue]

Replying to valendesigns:

@efegue the previous patch was a global deactivation of comment & trackback support for the page post type. What this patch does is set comment_status & ping_status to closed for new pages and post types that do not support those features. From a UX standpoint you can still check the option to display comments and trackbacks on a new page, so you will not need a developer to get comments back.

However, this patch does allow you to filter the default status for other post types. So a developer would be able to support comments and trackbacks in their custom post type, but have those features default to closed when a new post is created.

From my point of view this change is a lot less destructive, or potentially confusing, to the user. As well, it give us more control as developers to make decisions about our custom post types and how they support comments and trackbacks. And finally, we could standardize the default values with these new functions in other parts of Core.

Yup, you're absolutely right. I'll test the patch and let you know how it went. Thanks for the explanation.

[valendesigns]

The two new functions could potentially be combined into one with a second param to switch cases. Though is being extra DRY the correct approach in this situation if we could potentially loose clarity and what would the function be named if they were combined? Thought I'd put it out there for discussion at the very least.

A separate change we could make is to remove if ( 'page' === $post_type ) { from both functions and create an actual filter function for pages using the newly created filters. A bit of dogfooding in Core.

If anyone has other suggestions or even concerns with this approach please speak up soon as the enhancement deadline is looming.

[andg]

Replying to valendesigns:

@efegue the previous patch was a global deactivation of comment & trackback support for the page post type. What this patch does is set comment_status & ping_status to closed for new pages and post types that do not support those features. From a UX standpoint you can still check the option to display comments and trackbacks on a new page, so you will not need a developer to get comments back.

However, this patch does allow you to filter the default status for other post types. So a developer would be able to support comments and trackbacks in their custom post type, but have those features default to closed when a new post is created.

From my point of view this change is a lot less destructive, or potentially confusing, to the user. As well, it give us more control as developers to make decisions about our custom post types and how they support comments and trackbacks. And finally, we could standardize the default values with these new functions in other parts of Core.

+1

I was wondering, looking at the patch you've submitted, wouldn't the get_default_comment_status and get_default_ping_status filters be more clear if they had a $post_type parameter, otherwise one would be forced to use a global variable (correct me if I'm wrong).

Alternatively, the information about the post type being filtered could be included in the filter name itself, such as get_default_comment_status_$post_type.

In any case, this is a good solution to a very common problem, so thanks.

[valendesigns]

@andg I forgot to add the $post_type var to the filters. Duh! Nice catch lol.

[andg]

Replying to valendesigns:

@andg I forgot to add the $post_type var to the filters. Duh! Nice catch lol.

Glad to help ;)

[valendesigns]

The latest patch combines the two functions and adds a proper filter.

[rachelbaker]

Moved the logic for the different comment types into one public function get_default_comment_status that uses a new $comment_type param. Unit tests all pass.

[obenland]

In 33041:

Turn of comments for pages by default.

Pages are static content, for which comments are not expected out of the box.

Props valendesigns, rachelbaker.
Fixes #31168.

[helen]

We should write this up on Make/Core, especially given the change of behavior for new posts - we should be open to needing to change that default case.

[valendesigns]

@rachelbaker Thank you for the assist. Cheers!

[melchoyce]

Thanks everyone for making this happen. :) I am stoked!

[helen]

As I was working with Mel on the Make/Core post, I noticed that the hook ended up with a dynamic name: get_{$post_type}_default_comment_status. Dynamic hooks are neat, but not always easy to find, and there's usually a case to be made for the generic non-dynamic version. I think it'd be better to go with get_default_comment_status and pass the post type as context.

Also, do we need to do a general audit of get_option( 'default_comment_status' ) uses?

[obenland]

Replying to helen:

I noticed that the hook ended up with a dynamic name: get_{$post_type}_default_comment_status. Dynamic hooks are neat, but not always easy to find, and there's usually a case to be made for the generic non-dynamic version. I think it'd be better to go with get_default_comment_status and pass the post type as context.

I was actually thinking about that in my final review. I think we should definitely do that.

Also, do we need to do a general audit of get_option( 'default_comment_status' ) uses?

Yes we do.

[valendesigns]

I'll be at my desk shortly and will create a new patch.

[valendesigns]

The only place left in Core that uses default_comment_status and default_ping_status is in class-wp-xmlrpc-server.php and I'm not sure the best way to test the changes made to that file so I'm hesitant to make changes I can't verify. I've uploaded a patch that fixes the filter though.

[obenland]

Would we not replace all instanced of get_option( 'default_comment_status' ) and get_option( 'default_ping_status' )? There are a bit more of those.

[rachelbaker]

Looks like they are also used in redirect_post() and get_default_post_to_edit()

Replying to valendesigns:

The only place left in Core that uses default_comment_status and default_ping_status is in class-wp-xmlrpc-server.php and I'm not sure the best way to test the changes made to that file so I'm hesitant to make changes I can't verify. I've uploaded a patch that fixes the filter though.

[valendesigns]

I must have been doing a search earlier from inside the wp-includes directory and not from the root because I see a bunch more of them now. I'm replacing and running the unit tests. I'll upload a new patch soon.

[valendesigns]

I've switched over in various places to the new function and cleaned up the filter in 31168.7.diff​. Unit test all run as expected.

[obenland]

In 33054:

Use get_default_comment_status() globally.

Also makes the filter name static and passes the post type for context.

Props valendesigns.
Fixes #31168.

[nacin]

Is there a compelling reason for the filter to be a dynamic one in the form of get_{$post_type}_default_comment_status? I think this is more appropriate as apply_filters( "get_default_comment_status", $status, $post_type, $comment_type ).

We typically force specific, dynamic filters on developers in a few situations (these somewhat overlap/blur):

• when we find it to be overly generic, such as option (does not exist) vs option_{$option}
• when we find it to be overly dangerous, such as update_option (incorrectly using this filter took down .com, thus it was reverted there and #13836 was wontfix'd -- it was only added four years later when there was a compelling use case)
• when it'd be significantly easier to use the dynamic one, such as when it's used for manage_{$taxonomy}_custom_column
• when we use it as the underlying piece of an API, such as sanitize_option_{$option} (which is what register_setting() directly uses) or auth_post_meta_{$meta_key} (ditto for register_meta())

For something so minor, it seems annoying to need to add_filter() for each affected post type you may be trying to control. It's also a bit weird that it's broken down by *post* type and not (also?) by *comment* type, which is probably more expected here. I don't think there is a worry that someone will accidentally override the default comment status everywhere, unless that's actually exactly what they intend to do. As I write this, I wonder if this is worth keeping like this to avoid people stomping on random custom post types. But, most of our filters with a $post_type variable use it as an argument, and not as part of the dynamic name.

P.S. Could someone perhaps massage the above guidance into the handbook? :-D

[nacin]

Re-closing as now I see that this thought process was already laid out above, initially by @helen (and changed in [33054]), and I just missed it. This probably does belong in the handbook somewhere if anyone wants to take a swing. :-)

A few more thoughts on when to avoid dynamic hook names:

• When the static part of the dynamic hooks is so generically named that it can conflict with other dynamic (or static) hooks. This is a huge problem with some older hooks, but we're more wordy these days. (Not a major problem on this one.)
• As @helen said, it can be harder to find the hook, and also harder to take an add_filter() call in code you're reading and understand exactly what the actual filter is. (This could be a cool reverse-engineering tool to write...)

[nacin]

And finally, variables in a hook name should probably only ever be used at the end of the hook these days ("foo_bar_{$baz}", not "foo_{$bar}_baz"), to ease in finding things and to make it easier to avoid conflicts. I recognize many existing hooks in WordPress violate this.

[helen]

In 33122:

Move get_default_comment_status() to wp-includes/comment.php to sit alongside get_comment_statuses().

props nacin.
see #31168.