The checkmark to set subject fields as required is a poor solution to confusing quoted text ending up in the subject field by default.

It would be a lot more usable if the subject field could be properly filled with the first bit of text after any quotes. That would avoid any confusing subjects and be exactly the same from the behaviour the user is likely to expect.

Comments

junyor’s picture

This is really a rather difficult problem to solve. Before comment subjects are created from the comment body (when no subject was entered), lots of filtering and formatting is applied, including running all input filters. In other words, the "[quote]" stuff won't be in the comment body any more, making it very difficult to find the text that is unquoted. If you try to use the comment body prior to all this filtering and formatting, input filters will be bypassed and various other problems may occur.

I've been trying to think of a good solution for this, but haven't come up with anything yet. One possible solution is not to use the comment body, but the quoted item's title/subject. For instance, if you quote a node titled or comment with the subject "Foo bar", the comment subject would become "Re: Foo bar".

junyor’s picture

http://drupal.org/project/comment_subject implements the comment subject functionality I was talking about.

Below is some code I started working on to strip quotes. I abandoned it since I couldn't solve the input format problem. It won't work in its current state, but it should provide a good start if someone has some ideas.

/**
 * Implementation of hook_comment().
 */
function quote_comment($form_values, $op) {
  // We need to create the comment subject here so we can correctly strip out the quote
  // The following code is based on _comment_form_submit()
  if ($op == 'validate') {
    // Validate the comment's subject. If not specified, extract
    // one from the comment's body.
    if (trim($form_values['subject']) == '') {
      // The body may be in any format, so we:
      // 1) Strip out all HTML tags
      // 2) Convert entities back to plain-text.
      $subject = trim(decode_entities(strip_tags($form_values['comment'])));
      
      $start = strpos($subject, '[quote', 0);
      $length = strlen($subject);

      drupal_set_message("String length: $length");

      // make sure start isn't FALSE (not found) and that it's at least 29 characters in, which is the max subject trim length
      while ($start !== FALSE && $start < 29) {
        drupal_set_message("Start position: $start");

        // remove the quoted portion of the string
        $subject = substr_replace($subject, ' ', $start, $length);
        $start = strpos($subject, '[quote', $start);
      }

      $form_values['subject'] = trim(truncate_utf8(decode_entities(strip_tags(check_markup($subject, $form_values['format']))), 29, TRUE));
      // Edge cases where the comment body is populated only by HTML tags will
      // require a default subject.
      if ($form_values['subject'] == '') {
        $form_values['subject'] = t('(No subject)');
      }
    }

    drupal_set_message('Comment subject: "'. $form_values['subject'] .'"');
  }
}
ckng’s picture

Status: Active » Needs review
StatusFileSize
new1.7 KB

A patch to handle the auto subject generation.

This is how it works
- Insert form submission processing before the normal comment form_submit
- Strip the [quote ... [/quote] portion (not modifying the original comment)
- Set the subject title based on user comment body normally

ckng’s picture

StatusFileSize
new1.76 KB

Fixed a small bug where comment text was stripped when there is no quote.

Zen’s picture

Status: Needs review » Needs work

Visual review only: if ($start) will not work when $start is 0. Shouldn't strict equality be used? (Please see Junyor's code for an example).

I also don't believe that this will work in previews and with nested quotes which are not all that uncommon.

Thanks.

Zen’s picture

Version: 5.x-1.3 » 7.x-1.x-dev
ivnish’s picture

Issue summary: View changes
Status: Needs work » Closed (outdated)