WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#9900 closed defect (bug) (fixed)

allow larger value for thread_comments_depth_max

Reported by: hailin Owned by:
Milestone: 2.9 Priority: normal
Severity: normal Version: 2.7
Component: Comments Keywords: commit early
Focuses: Cc:

Description

Current thread_comments_depth_max is set to be at most 10.
In some cases, users want deeper levels, such as 12, they will
do
add_filter('pre_option_thread_comments_depth', create_function(, 'return 12;'));

It causes some confusion because the in options-discussion.php

if ( get_option('thread_comments_depth') == $i )

thread_comments_depth .= " selected='selected'";

$i will be at most 10, so it couldn't find any matching entry,
so the dropdown menu will display 1. It will always display 1 even if the user tries to change the value to 8, or 10.

So I think it's better to allow a larger max value to avoid this confusion.


Attachments (2)

9900_threaded_comments.diff (1.6 KB) - added by hailin 5 years ago.
patch
9900_threaded_comments.2.diff (1.2 KB) - added by hailin 5 years ago.
revised patch

Download all attachments as: .zip

Change History (14)

hailin5 years ago

patch

comment:1 hailin5 years ago

also changed
Break comments into pages with %1$s comments per page
To:
Break comments into pages with %1$s top level comments per page

Because we are really paging by top level comments here. I've heard users actually count the exact number of comments on each page, and complain "it's not exactly the configured number!" So it's better to be precise here.

comment:2 hailin5 years ago

it's a safe fix

comment:3 Denis-de-Bernardy5 years ago

  • Component changed from General to Comments
  • Milestone changed from Unassigned to 2.8

For information, there is a related ticket somewhere. In it, I suggest changing the behavior altogether: allow infinite depth, and change the walker in such a way to as it enforces a max depth during the display.

This allows to not worry about the depth when changing themes, among many things.

comment:4 hailin5 years ago

Philosophically, I endorse Perl's "no unnecessary limits".

However, from usability perspective, it's better to enforce a max level constraint.
Believe it or not, some power users with >500 comments per post have been using 12 levels for a long time. So I think increase it to 20 is a reasonable alternative
for the near future.

comment:5 westi5 years ago

  • Milestone changed from 2.8 to 2.9
  • Version set to 2.7

So you want to change the default here because people are using the wrong filter to change this?

If they want the display to be correct when they have forced the value using the pre_option hook they can add another filter in which sets the max to 12.

Not sure this warrants going into 2.8, I don't want to change translated strings during beta unless really necessary.

Moving to 2.9 for now.

comment:6 hailin5 years ago

Ok, the max depth can be modified via filter 'thread_comments_depth_max'.
That works equally well. I've recommended affected users to use the filter instead.

The updated patch contains only a minor fix to stress "top level" in the "break .." statement. It clarifies the meaning a little bit.

hailin5 years ago

revised patch

comment:7 Denis-de-Bernardy5 years ago

  • Keywords commit added
  • Milestone changed from 2.9 to 2.8

comment:8 ryan5 years ago

(In [11479]) Revert [11475]. Keep those strings frozen for 2.8. see #9900

comment:9 ryan5 years ago

  • Milestone changed from 2.8 to 2.9

comment:10 Denis-de-Bernardy5 years ago

  • Keywords early added

comment:11 Denis-de-Bernardy5 years ago

still applies clean

comment:12 westi5 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [11591]) Improve description of comments_per_page option. Fixes #9900 props hailin.

Note: See TracTickets for help on using tickets.