I think this may break existing code which calls getActionList() or setActionList(). Consider this code:

$curtain->addAction("A");
$curtain->setActionList("B");

Before, this would render "B" (the "set" overrides the action list) -- or maybe it would fatal? Now, it will render "B", "A".

Consider this code:

$curtain->addAction("A");
$view = array($curtain, $curtain);

Before, each curtain would render with "A". Now, the first curtain will render with "A". The second curtain will render with "A", "A", because we add "A" each time getTagContent() is called.

Consider this code:

$curtain->setActionList("B");

Before, this would render "B". Now, it won't render anything, because $this->actions will be empty.

It's not clear to me exactly what problem this is solving: why did you need to add actions/an action list before? From the code here, it doesn't immediately look like anything here requires it. I think we can probably find a better fix here without creating these weird situations where mixtures of add/set do surprising things.

ded

The main issue is I don't want to append an action list if it's empty. I just get a single <ul>, which leads to CSS wonkiness.

Does this work as a "fix"? Still a little wonky but doesn't make things worse, I think.

diff --git a/src/view/layout/PhabricatorActionListView.php b/src/view/layout/PhabricatorActionListView.php
index faf30555eb..134c336735 100644
--- a/src/view/layout/PhabricatorActionListView.php
+++ b/src/view/layout/PhabricatorActionListView.php
@@ -16,6 +16,10 @@ final class PhabricatorActionListView extends AphrontTagView {
   }
 
   protected function getTagName() {
+    if (!$this->actions) {
+      return null;
+    }
+
     return 'ul';
   }