From 976a21f6584bda13289cee12599d29cb3126dbd4 Mon Sep 17 00:00:00 2001 From: Valerio Bozzolan Date: Fri, 19 May 2023 22:05:26 +0200 Subject: [PATCH] AphrontFormControl: fix regression for some specific Captions Summary: Fix a specific problem when visiting some specific pages like this one: /auth/config/edit/?provider=PhabricatorPhabricatorAuthProvider: Regression introduced in: 562d36ef5f9e83dba5e3ca0244f9c4af9e1f7b2b Stack trace: [Fri May 19 14:23:35.506028 2023] [php7:notice] [pid 20439] [client 127.0.0.1:39692] [2023-05-19 14:23:35] EXCEPTION: (InvalidArgumentException) Call to phutil_nonempty_string() expected null or a string, got: PhutilSafeHTML. at [/src/utils/utils.php:2127] [Fri May 19 14:23:35.506647 2023] [php7:notice] [pid 20439] [client 127.0.0.1:39692] arcanist(head=arcpatch-D25049, ref.master=c14785c3795c, ref.arcpatch-D25049=1b6412c24640), phorge(head=arcpatch-D25216_1, ref.master=2df7ea13a387, ref.arcpatch-D25216_1=02b40a9e25eb) [Fri May 19 14:23:35.506661 2023] [php7:notice] [pid 20439] [client 127.0.0.1:39692] #0 <#2> phutil_nonempty_string(PhutilSafeHTML) called at [/src/view/form/control/AphrontFormControl.php:206] [Fri May 19 14:23:35.506665 2023] [php7:notice] [pid 20439] [client 127.0.0.1:39692] #1 <#2> phutil_tag(string, array, array) called at [/src/view/form/PHUIFormLayoutView.php:54] [Fri May 19 14:23:35.506667 2023] [php7:notice] [pid 20439] [client 127.0.0.1:39692] #2 <#2> PHUIFormLayoutView::render() called at [/src/view/form/AphrontFormView.php:160] We keep 100% backward compatibility, reproducing the implicit cast happening automatically before PHP 8.1 It has also been simplified the non-empty check since that is possible after casting to string. Closes T15404 Test Plan: - Visit the page mentioned in the commit summary - no longer explodes - you still don't see empty Captions - you still see populated Captions - you still see HTML rendered for populated Captions Reviewers: O1 Blessed Committers, avivey Reviewed By: O1 Blessed Committers, avivey Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno Maniphest Tasks: T15404 Differential Revision: https://we.phorge.it/D25231 --- src/view/form/control/AphrontFormControl.php | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/view/form/control/AphrontFormControl.php b/src/view/form/control/AphrontFormControl.php index d891636230..83e0207241 100644 --- a/src/view/form/control/AphrontFormControl.php +++ b/src/view/form/control/AphrontFormControl.php @@ -56,11 +56,22 @@ abstract class AphrontFormControl extends AphrontView { return $this->label; } + /** + * Set the Caption + * The Caption shows a tip usually nearby the related input field. + * @param string|PhutilSafeHTML|null + * @return self + */ public function setCaption($caption) { $this->caption = $caption; return $this; } + /** + * Get the Caption + * The Caption shows a tip usually nearby the related input field. + * @return string|PhutilSafeHTML|null + */ public function getCaption() { return $this->caption; } @@ -203,7 +214,11 @@ abstract class AphrontFormControl extends AphrontView { $custom_class .= ' aphront-form-control-nolabel'; } - if (phutil_nonempty_string($this->getCaption())) { + // The Caption can be stuff like PhutilSafeHTML and other objects that + // can be casted to a string. After this cast we have never null. + $has_caption = phutil_string_cast($this->getCaption()) !== ''; + + if ($has_caption) { $caption = phutil_tag( 'div', array('class' => 'aphront-form-caption'),