From fb5dec4c036b57c7cb780c690c09c56568c29d50 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 30 May 2019 12:23:51 -0700 Subject: [PATCH] Require valid comments to contain at least one non-whitespace character Summary: See downstream . This is very marginal, but we currently allow comments consisting of //only// whitespace. These are probably always mistakes, so treat them like completely empty comments. (We intentionally do not trim leading or trailing whitespace from comments when posting them becuase leading spaces can be used to trigger codeblock formatting.) Test Plan: - Posted empty, nonempty, and whitespace-only comments. - Whitespace-only comments now have the same behavior as truly empty comments (e.g., do not actually generate a transaction). Reviewers: amckinley Reviewed By: amckinley Differential Revision: https://secure.phabricator.com/D20562 --- .../storage/PhabricatorApplicationTransaction.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index 9850d66a7f..775d59d4db 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -127,7 +127,19 @@ abstract class PhabricatorApplicationTransaction } public function hasComment() { - return $this->getComment() && strlen($this->getComment()->getContent()); + if (!$this->getComment()) { + return false; + } + + $content = $this->getComment()->getContent(); + + // If the content is empty or consists of only whitespace, don't count + // this as comment. + if (!strlen(trim($content))) { + return false; + } + + return true; } public function getComment() {