Allow users to unset "Editor", tailor short error messages properly on settings forms

Summary:
Ref T11098.

  - Allow "Editor" to be set to the empty string.
  - Don't match a validation error to a field unless the actual settings for the field and error match.

Test Plan:
  - Tried to set "Editor" to "", success.
  - Tried to set "Editor" to "javascript://", only that field got marked "Invalid".

Reviewers: avivey, chad

Reviewed By: chad

Maniphest Tasks: T11098

Differential Revision: https://secure.phabricator.com/D16051

src/applications/settings/editor/PhabricatorSettingsEditEngine.php

@@ -194,4 +194,43 @@ final class PhabricatorSettingsEditEngine
     return $fields;
   }
 
+  protected function getValidationExceptionShortMessage(
+    PhabricatorApplicationTransactionValidationException $ex,
+    PhabricatorEditField $field) {
+
+    // Settings fields all have the same transaction type so we need to make
+    // sure the transaction is changing the same setting before matching an
+    // error to a given field.
+    $xaction_type = $field->getTransactionType();
+    if ($xaction_type == PhabricatorUserPreferencesTransaction::TYPE_SETTING) {
+      $property = PhabricatorUserPreferencesTransaction::PROPERTY_SETTING;
+
+      $field_setting = idx($field->getMetadata(), $property);
+      foreach ($ex->getErrors() as $error) {
+        if ($error->getType() !== $xaction_type) {
+          continue;
+        }
+
+        $xaction = $error->getTransaction();
+        if (!$xaction) {
+          continue;
+        }
+
+        $xaction_setting = $xaction->getMetadataValue($property);
+        if ($xaction_setting != $field_setting) {
+          continue;
+        }
+
+        $short_message = $error->getShortMessage();
+        if ($short_message !== null) {
+          return $short_message;
+        }
+      }
+
+      return null;
+    }
+
+    return parent::getValidationExceptionShortMessage($ex, $field);
+  }
+
 }

src/applications/settings/setting/PhabricatorEditorSetting.php

@@ -39,6 +39,10 @@ final class PhabricatorEditorSetting
   }
 
   public function validateTransactionValue($value) {
+    if (!strlen($value)) {
+      return;
+    }
+
     $ok = PhabricatorHelpEditorProtocolController::hasAllowedProtocol($value);
     if ($ok) {
       return;

src/applications/transactions/editengine/PhabricatorEditEngine.php

@@ -1004,12 +1004,7 @@ abstract class PhabricatorEditEngine
         $validation_exception = $ex;
 
         foreach ($fields as $field) {
-          $xaction_type = $field->getTransactionType();
-          if ($xaction_type === null) {
-            continue;
-          }
-
-          $message = $ex->getShortMessage($xaction_type);
+          $message = $this->getValidationExceptionShortMessage($ex, $field);
           if ($message === null) {
             continue;
           }
@@ -2049,6 +2044,18 @@ abstract class PhabricatorEditEngine
       ->setHref($item_uri);
   }
 
+  protected function getValidationExceptionShortMessage(
+    PhabricatorApplicationTransactionValidationException $ex,
+    PhabricatorEditField $field) {
+
+    $xaction_type = $field->getTransactionType();
+    if ($xaction_type === null) {
+      return null;
+    }
+
+    return $ex->getShortMessage($xaction_type);
+  }
+
   protected function getCreateNewObjectPolicy() {
     return PhabricatorPolicies::POLICY_USER;
   }

src/applications/transactions/error/PhabricatorApplicationTransactionValidationError.php

@@ -26,7 +26,7 @@ final class PhabricatorApplicationTransactionValidationError
   }
 
   public function getTransaction() {
-    return $this->tranaction;
+    return $this->transaction;
   }
 
   public function getShortMessage() {